Skip to content

feat(registry-importer,news-greenhouse): DOMA-12984 api new urls#7283

Merged
vovaaxeapolla merged 3 commits intomainfrom
feat/registry-importer-news-greenhouse/DOMA-12984/new-urls
Mar 5, 2026
Merged

feat(registry-importer,news-greenhouse): DOMA-12984 api new urls#7283
vovaaxeapolla merged 3 commits intomainfrom
feat/registry-importer-news-greenhouse/DOMA-12984/new-urls

Conversation

@vovaaxeapolla
Copy link
Contributor

@vovaaxeapolla vovaaxeapolla commented Mar 2, 2026

Not intended to be merged right now. New api urls are unstable

Summary by CodeRabbit

  • Chores
    • Updated submodule references across multiple components and apps (internal dependency pointers updated).
    • No user-facing, configuration, or functional changes observed; no changes to public APIs.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0ad6f4e-0651-482b-8c7c-e5532b6b2482

📥 Commits

Reviewing files that changed from the base of the PR and between bc077e7 and 01901f2.

📒 Files selected for processing (3)
  • .helm
  • apps/news-greenhouse
  • apps/registry-importer
✅ Files skipped from review due to trivial changes (2)
  • apps/news-greenhouse
  • apps/registry-importer
🚧 Files skipped from review as they are similar to previous changes (1)
  • .helm

📝 Walkthrough

Walkthrough

This pull request updates git submodule commit pointers in three locations (.helm, apps/news-greenhouse, apps/registry-importer) with no code, configuration, API, or behavioral changes.

Changes

Cohort / File(s) Summary
Submodule: .helm
\.helm
Updated submodule commit reference (commit pointer change only; no functional/config/API changes).
Submodule: apps/news-greenhouse
apps/news-greenhouse
Updated submodule commit reference (commit pointer change only; no functional/config/API changes).
Submodule: apps/registry-importer
apps/registry-importer
Updated submodule commit reference (commit pointer change only; no functional/config/API changes).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐇 I nudged three commits along the trail,
Tiny hops in code, a whispering tail,
No lines rewritten, no tests to chase,
Just version footprints in their place. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references updates to registry-importer and news-greenhouse with new API URLs, which aligns with the submodule reference updates shown in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/registry-importer-news-greenhouse/DOMA-12984/new-urls

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Confidence Score: 2/5

  • PR should not be merged—author explicitly states API URLs are unstable and PR is not ready.
  • While the PR's structure (submodule pointer bumps) is mechanically sound, the author's clear warning that this PR is not intended for merge due to unstable API URLs takes precedence. The three submodule changes reference upstream repositories where the actual API URL logic lives and cannot be fully reviewed here. The internal code quality and correctness of the API URL implementations must be verified in their respective repositories (condo-helm, condo-news-greenhouse, condo-registry-importer) before this PR is considered safe. Low confidence reflects both the incomplete visibility into submodule internals and the author's own caution.
  • .helm, apps/news-greenhouse, apps/registry-importer — all three submodule changes require thorough testing and review in their respective repositories before merging.

Last reviewed commit: bc077e7

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/registry-importer`:
- Line 1: Block merging this PR until the submodule SHA for
apps/registry-importer (1d79ea10b0009b4b9a60e024894a44be442e4c9c) is explicitly
verified as stable: add a draft/hold label or CI gate, require the author to
confirm the intended commit SHA and provide verification (e.g., git show output,
signed tag, or internal release note) in the PR description or a follow-up
comment, and update CI/merge checks to fail unless the verification comment or
approved checklist entry referencing that exact SHA is present.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 888ae96 and 7528ec9.

📒 Files selected for processing (3)
  • .helm
  • apps/news-greenhouse
  • apps/registry-importer

@@ -1 +1 @@
Subproject commit a6fe11be47f72994b6cde5941aaf47501eacbd63
Subproject commit 1d79ea10b0009b4b9a60e024894a44be442e4c9c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Confirm submodule URL for apps/registry-importer
echo "=== Step 1: Submodule URL ==="
git config -f .gitmodules --get submodule.apps/registry-importer.url 2>&1 || echo "Submodule config not found"

# 2) Confirm the gitlink SHA currently pointed to by this PR
echo -e "\n=== Step 2: Current gitlink SHA ==="
git ls-tree HEAD apps/registry-importer 2>&1 || echo "No gitlink found at HEAD"

# 3) Verify the SHA exists on the submodule remote
echo -e "\n=== Step 3: Checking if SHA exists on remote ==="
SUBMODULE_URL="$(git config -f .gitmodules --get submodule.apps/registry-importer.url 2>&1)" || echo "Could not get URL"
if [ -n "${SUBMODULE_URL:-}" ]; then
  TARGET_SHA="1d79ea10b0009b4b9a60e024894a44be442e4c9c"
  echo "Checking for SHA: $TARGET_SHA"
  git ls-remote "$SUBMODULE_URL" | grep -i "$TARGET_SHA" || echo "Target SHA not found on remote"
else
  echo "Skipping remote check due to missing URL"
fi

# 4) (Optional) Show commit subject from remote by shallow-fetching metadata
echo -e "\n=== Step 4: Commit info (if accessible) ==="
if [ -n "${SUBMODULE_URL:-}" ]; then
  tmpdir="$(mktemp -d)"
  trap "rm -rf $tmpdir" EXIT
  git -C "$tmpdir" init -q 2>&1 || true
  git -C "$tmpdir" remote add origin "$SUBMODULE_URL" 2>&1 || true
  git -C "$tmpdir" fetch --depth=1 origin "$TARGET_SHA" 2>&1 || echo "Could not fetch shallow commit"
  git -C "$tmpdir" show --no-patch --pretty=fuller FETCH_HEAD 2>&1 || echo "Could not show commit"
else
  echo "Skipping commit fetch due to missing URL"
fi

Repository: open-condo-software/condo

Length of output: 926


🏁 Script executed:

#!/bin/bash
set -euo pipefail

SUBMODULE_URL="git@github.com:open-condo-software/condo-registry-importer.git"
TARGET_SHA="1d79ea10b0009b4b9a60e024894a44be442e4c9c"

echo "=== Attempt 1: Full ls-remote output (first 20 refs) ==="
git ls-remote "$SUBMODULE_URL" 2>&1 | head -20 || echo "Failed to access remote"

echo -e "\n=== Attempt 2: Check if SHA is reachable via any ref ==="
git ls-remote "$SUBMODULE_URL" | wc -l

echo -e "\n=== Attempt 3: Search for exact SHA anywhere in refs ==="
git ls-remote "$SUBMODULE_URL" | grep -c "$TARGET_SHA" || echo "0 matches found"

echo -e "\n=== Attempt 4: List remote branches and tags (https fallback) ==="
# Try HTTPS URL as fallback in case SSH is the issue
git ls-remote "https://github.com/open-condo-software/condo-registry-importer.git" 2>&1 | head -30 || echo "HTTPS access also failed"

Repository: open-condo-software/condo

Length of output: 370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking .gitmodules file ==="
cat .gitmodules 2>/dev/null | grep -A 3 "registry-importer" || echo "No .gitmodules found"

echo -e "\n=== Checking git config ==="
git config --get-regexp "registry-importer" || echo "No config found"

echo -e "\n=== List all submodules ==="
git config --file .gitmodules --name-only --get-regexp path | head -20

echo -e "\n=== Check current HEAD and recent commits ==="
git log --oneline -n 10 2>&1 | head -15

echo -e "\n=== Check if apps/registry-importer directory exists in working tree ==="
if [ -d "apps/registry-importer" ]; then
  echo "Directory exists"
  ls -la apps/registry-importer/ 2>&1 || echo "Cannot list contents"
else
  echo "Directory does not exist in working tree (expected for submodule)"
fi

echo -e "\n=== Verify this is a gitlink (submodule pointer) ==="
file apps/registry-importer 2>&1 || echo "File command failed"
</script>

Repository: open-condo-software/condo

Length of output: 1632


🌐 Web query:

condo-registry-importer 1d79ea10b0009b4b9a60e024894a44be442e4c9c github

💡 Result:

  • A GitHub repository search for condo-registry-importer returns 0 results (no public repo with that exact name found). [1]
  • A GitHub code search for the SHA 1d79ea10b0009b4b9a60e024894a44be442e4c9c can’t be validated from the public web view because GitHub requires you to sign in to use code search. [2]

If you share the expected owner/org (e.g., org/repo) or a direct URL, I can narrow it down further.

Sources: [1], [2]


Block merge until this submodule SHA is explicitly verified as stable.

Line 1 moves apps/registry-importer to commit 1d79ea10b0009b4b9a60e024894a44be442e4c9c, but this PR is described as using unstable API URLs. The target repository is private and the SHA cannot be validated from public sources. Gate this behind a draft/hold policy and verify the target SHA is intentional and release-safe before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/registry-importer` at line 1, Block merging this PR until the submodule
SHA for apps/registry-importer (1d79ea10b0009b4b9a60e024894a44be442e4c9c) is
explicitly verified as stable: add a draft/hold label or CI gate, require the
author to confirm the intended commit SHA and provide verification (e.g., git
show output, signed tag, or internal release note) in the PR description or a
follow-up comment, and update CI/merge checks to fail unless the verification
comment or approved checklist entry referencing that exact SHA is present.

Copy link
Member

@toplenboren toplenboren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but in domains/greendom/constants.js it is best to add a comment indicating that v2: .../v1/... is OK

@vovaaxeapolla vovaaxeapolla force-pushed the feat/registry-importer-news-greenhouse/DOMA-12984/new-urls branch from 7528ec9 to bc077e7 Compare March 5, 2026 07:32
@vovaaxeapolla vovaaxeapolla force-pushed the feat/registry-importer-news-greenhouse/DOMA-12984/new-urls branch from bc077e7 to 01901f2 Compare March 5, 2026 09:50
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 5, 2026

@vovaaxeapolla vovaaxeapolla merged commit 92f9284 into main Mar 5, 2026
163 of 167 checks passed
@vovaaxeapolla vovaaxeapolla deleted the feat/registry-importer-news-greenhouse/DOMA-12984/new-urls branch March 5, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants