Skip to content

fix(ci): update .main.commit for non-dev mcore branches - #3213

Merged
ko3n1g merged 1 commit into
mainfrom
ko3n1g/fix/update-main-commit-release-branches
Apr 8, 2026
Merged

fix(ci): update .main.commit for non-dev mcore branches#3213
ko3n1g merged 1 commit into
mainfrom
ko3n1g/fix/update-main-commit-release-branches

Conversation

@ko3n1g

@ko3n1g ko3n1g commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When bumping on release branches (e.g. r0.4.0 with mcore-branch=mcore-core_r0.17.0), the mcore-branch input matches neither "main" nor "dev", so .main.commit was never written
  • The unit test test_main_commit_matches_submodule then fails because the bumped submodule commit no longer matches .main.commit
  • Fix: write .main.commit for all non-dev branches (covers both main and release branches)

Before:

if [[ "$TARGET_BRANCH" == "main" ]]; then
    echo $MLM_COMMIT > .main.commit
elif [[ "$TARGET_BRANCH" == "dev" ]]; then
    echo $MLM_COMMIT > .dev.commit
fi

After:

if [[ "$TARGET_BRANCH" == "dev" ]]; then
    echo $MLM_COMMIT > .dev.commit
else
    echo $MLM_COMMIT > .main.commit
fi

Test plan

  • Verify test_main_commit_matches_submodule passes on next bump run for r0.4.0

Summary by CodeRabbit

Release Notes

  • Chores
    • Updated internal dependency management workflow to improve commit tracking across different release branches.

Note: This release contains only internal infrastructure improvements with no direct impact on end-user functionality.

When bumping on release branches (e.g. r0.4.0 with mcore-core_r0.17.0),
the mcore-branch input matches neither "main" nor "dev", so .main.commit
was never written. The unit test test_main_commit_matches_submodule then
fails because the submodule no longer matches .main.commit.

Fix: write .main.commit for all non-dev branches (main + release branches).
Signed-off-by: oliver könig <okoenig@nvidia.com>
@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dc237712-01d4-4afc-a4cc-f12ba9ce38f2

📥 Commits

Reviewing files that changed from the base of the PR and between 07c9920 and c52778e.

📒 Files selected for processing (1)
  • .github/workflows/_update_dependencies.yml

📝 Walkthrough

Walkthrough

The workflow file's conditional logic for writing Megatron-LM submodule commit hashes has been restructured. The logic now checks if TARGET_BRANCH is dev to write to .dev.commit, and otherwise writes to .main.commit. The previous implementation had separate conditions for main and dev branches.

Changes

Cohort / File(s) Summary
Workflow Branch Targeting
.github/workflows/_update_dependencies.yml
Modified conditional logic for submodule commit hash file writing based on TARGET_BRANCH value; changed from separate if/elif conditions to a consolidated check for dev branch with a default case.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating .main.commit to be written for non-dev branches (mcore branches) instead of only for main.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Test Results For Major Changes ✅ Passed Minor CI workflow bug fix with documented test plan verifying test_main_commit_matches_submodule passes on next bump run.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ko3n1g/fix/update-main-commit-release-branches

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

@ko3n1g
ko3n1g enabled auto-merge (squash) April 8, 2026 11:15
@ko3n1g
ko3n1g merged commit c0fe76d into main Apr 8, 2026
52 checks passed
@ko3n1g
ko3n1g deleted the ko3n1g/fix/update-main-commit-release-branches branch April 8, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants