Partial-update rollback: atomic submodule commit with failure summary.#32
Open
AuraMindNest wants to merge 3 commits into
Open
Partial-update rollback: atomic submodule commit with failure summary.#32AuraMindNest wants to merge 3 commits into
AuraMindNest wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughShared shell helpers now track submodule-fatal and open-PR skip state, encode and decode submodule names as JSON, and print consolidated summaries. The translation workflows pass updated submodules from ChangesSubmodule translation workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/start-translation.yml (1)
326-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSkip Weblate when local finalization fails.
Line 339 triggers Weblate even if
finalize_translations_localfailed andrcis non-zero, which can notify Weblate before the local branch state is safely pushed.Proposed guard
weblate_rc=0 - trigger_weblate "$WEBLATE_URL" "$WEBLATE_TOKEN" "$LIBS_REF" "$exts_json" "$LANG_CODE" || weblate_rc=$? + if [[ $rc -eq 0 ]]; then + trigger_weblate "$WEBLATE_URL" "$WEBLATE_TOKEN" "$LIBS_REF" "$exts_json" "$LANG_CODE" || weblate_rc=$? + else + echo "Skipping Weblate because local branch finalization failed." >&2 + fi end_phase🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/start-translation.yml around lines 326 - 346, The workflow currently calls trigger_weblate even when finalize_translations_local fails and rc is non-zero, so guard the Weblate phase behind a successful finalization. Update the start-translation logic around finalize_translations_local, trigger_weblate, and the rc/weblate_rc aggregation so Weblate is only triggered when rc remains 0, while preserving the existing exit_rc handling for failures.
🧹 Nitpick comments (1)
.github/workflows/assets/lib.sh (1)
331-355: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDefensive-guard inconsistency for
SUBMODULE_FATAL/UPDATES/OPEN_PR_SKIP.The function guards
META_MISSING,ORG_REPO_MISSING,NO_DOC_PATHS, andREPO_EXISTS_SKIPwith[[ ${VAR+set} ]], but iterates/expandsSUBMODULE_FATAL,UPDATES, andOPEN_PR_SKIPdirectly. These are only initialized viainit_translation_state, so calling the summary before init underset -u(older bash) would trip an unbound-variable error for the very arrays that aren't guarded. Either guard all of them or none for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/assets/lib.sh around lines 331 - 355, The summary function has inconsistent defensive guards: `print_submodule_processing_summary` protects `META_MISSING`, `ORG_REPO_MISSING`, `NO_DOC_PATHS`, and `REPO_EXISTS_SKIP`, but reads `SUBMODULE_FATAL`, `UPDATES`, and `OPEN_PR_SKIP` directly. Update the function to use the same `[[ ${VAR+set} ]]`-style presence checks or equivalent safe defaults for all array references in `print_submodule_processing_summary`, so it won’t fail under `set -u` if called before `init_translation_state`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/add-submodules.yml:
- Around line 210-218: Ensure finalize_translations_repo stops on the first
failure instead of continuing after a failed finalize_translations_master or
finalize_translations_local call; update that function so it returns immediately
with the failing status, and then have the caller in add-submodules.yml rely on
the returned rc rather than allowing later successful steps to overwrite it.
In @.github/workflows/start-translation.yml:
- Around line 185-198: The start-translation workflow currently writes
updated_submodules before finalize_translations_master runs, which lets
start-local act on branches even if mirror finalization fails. Move the
updated_submodules output write in the workflow step so it only happens after
finalize_translations_master succeeds, and keep the export guarded by the same
rc/exit_rc flow used in this phase. Use the updated_json, GITHUB_OUTPUT, and
finalize_translations_master step to locate the change, and ensure the output
stays empty when finalization fails.
---
Outside diff comments:
In @.github/workflows/start-translation.yml:
- Around line 326-346: The workflow currently calls trigger_weblate even when
finalize_translations_local fails and rc is non-zero, so guard the Weblate phase
behind a successful finalization. Update the start-translation logic around
finalize_translations_local, trigger_weblate, and the rc/weblate_rc aggregation
so Weblate is only triggered when rc remains 0, while preserving the existing
exit_rc handling for failures.
---
Nitpick comments:
In @.github/workflows/assets/lib.sh:
- Around line 331-355: The summary function has inconsistent defensive guards:
`print_submodule_processing_summary` protects `META_MISSING`,
`ORG_REPO_MISSING`, `NO_DOC_PATHS`, and `REPO_EXISTS_SKIP`, but reads
`SUBMODULE_FATAL`, `UPDATES`, and `OPEN_PR_SKIP` directly. Update the function
to use the same `[[ ${VAR+set} ]]`-style presence checks or equivalent safe
defaults for all array references in `print_submodule_processing_summary`, so it
won’t fail under `set -u` if called before `init_translation_state`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e2cdf0e-57d6-4561-9c1b-012091db11f2
📒 Files selected for processing (7)
.github/workflows/add-submodules.yml.github/workflows/assets/env.sh.github/workflows/assets/lib.sh.github/workflows/assets/translation.sh.github/workflows/start-translation.ymltests/helpers/common.bashtests/test_lib.bats
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #30.
Summary by CodeRabbit
start-localnow runs only forlibs/*submodules that successfully updated during mirror synchronization.