Rename child-manifests to sub-manifests - #1027
Conversation
Sorry, something went wrong.
|
Warning Rate limit exceeded@spoorcc has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📥 CommitsReviewing files that changed from the base of the PR and between 6d8d564 and 3b1eff9. 📒 Files selected for processing (10)
WalkthroughSystematic rename of "child-manifests" to "sub-manifests" across code, docs, tests, and feature files. Public symbols renamed (e.g., get_childmanifests → get_submanifests, check_child_manifests → check_sub_manifests). No functional control-flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labelsdevelopment, documentation Suggested reviewers
❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches 🧪 Generate unit tests (beta)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)dfetch/manifest/parse.py (1)🤖 Prompt for all review comments with AI agentsdfetch/commands/common.py (1)100-100: Minor: Use singular form in log message.
The log message refers to a single path but uses the plural form "sub-manifests".
Proposed fix🤖 Prompt for AI Agents- logger.debug(f"Found sub-manifests {path}") + logger.debug(f"Found sub-manifest {path}")Verify each finding against the current code and only fix it if needed. In `@dfetch/manifest/parse.py` at line 100, Change the log message string used in the logger.debug call that currently reads "Found sub-manifests {path}" to the singular form "Found sub-manifest {path}" (the call using logger.debug and the variable path in parse.py); update the message text only so it correctly reflects a single path.25-27: Use a precomputed set for remote URL membership checks.
Line 25 rebuilds a list on every subproject iteration and reuses project as the inner variable name, which hurts readability and scales poorly.
♻️ Suggested refactor🤖 Prompt for AI Agentsdef check_sub_manifests(manifest: Manifest, project: ProjectEntry) -> None: @@ - for submanifest in get_submanifests(skip=[manifest.path]): + manifest_remote_urls = { + parent_project.remote_url for parent_project in manifest.projects + } + for submanifest in get_submanifests(skip=[manifest.path]): recommendations: list[ProjectEntry] = [] for subproject in submanifest.projects: - if subproject.remote_url not in [ - project.remote_url for project in manifest.projects - ]: + if subproject.remote_url not in manifest_remote_urls: recommendations.append(subproject.as_recommendation())Verify each finding against the current code and only fix it if needed. In `@dfetch/commands/common.py` around lines 25 - 27, Precompute a set of existing remote URLs before iterating subproject to avoid rebuilding the list and inner-variable shadowing; e.g., create a set from manifest.projects' remote_url values (use a name like existing_remote_urls) and then replace the condition "if subproject.remote_url not in [project.remote_url for project in manifest.projects]" with a membership check against that set (if subproject.remote_url not in existing_remote_urls), ensuring you remove the inner "project" list comprehension to improve readability and performance.
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.rst`:
- Line 5: Replace the placeholder issue reference "(`#0`)" with the correct PR
number "(`#1027`)" in the CHANGELOG.rst entry that renames "child-manifests" to
"sub-manifests" so the changelog reads "* Rename child-manifests to
sub-manifests in documentation and code (`#1027`)"; update only the issue number
token in that line.
---
Nitpick comments:
In `@dfetch/commands/common.py`:
- Around line 25-27: Precompute a set of existing remote URLs before iterating
subproject to avoid rebuilding the list and inner-variable shadowing; e.g.,
create a set from manifest.projects' remote_url values (use a name like
existing_remote_urls) and then replace the condition "if subproject.remote_url
not in [project.remote_url for project in manifest.projects]" with a membership
check against that set (if subproject.remote_url not in existing_remote_urls),
ensuring you remove the inner "project" list comprehension to improve
readability and performance.
In `@dfetch/manifest/parse.py`:
- Line 100: Change the log message string used in the logger.debug call that
currently reads "Found sub-manifests {path}" to the singular form "Found
sub-manifest {path}" (the call using logger.debug and the variable path in
parse.py); update the message text only so it correctly reflects a single path.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between d1524cd and c18cfcc.
📒 Files selected for processing (10)- CHANGELOG.rst
- dfetch/commands/check.py
- dfetch/commands/common.py
- dfetch/commands/update.py
- dfetch/manifest/parse.py
- doc/internal.rst
- features/updated-project-has-dependencies.feature
- tests/test_check.py
- tests/test_manifest.py
- tests/test_update.py
Sorry, something went wrong.
Rename child-manifest to sub-manifest to make concepts less confusing.
Summary by CodeRabbit