- Notifications
You must be signed in to change notification settings - Fork 203
chore: Update dev branches with merge #3474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the previous rebase-based dev-branch updates with a merge-based workflow to avoid repeat conflicts caused by squashed commits. Key changes include removing the rebase inputs, adding a commit-count check and an automated git merge
, and replacing the PR creation step with direct pushes and Slack notifications.
Comments suppressed due to low confidence (2)
.github/workflows/update-dev-branches.yml:54
- To ensure you're counting up-to-date master commits, run
git fetch origin master
(or a full fetch) before computingcommits_to_merge
, otherwiseorigin/master
may be stale from earlier steps.
commits_to_merge=$(git rev-list --count origin/${{ matrix.branch }}..origin/master)
.github/workflows/update-dev-branches.yml:64
- [nitpick] Consider moving these detailed manual resolution steps to an external documentation page and linking to it here, so the workflow file stays concise and easier to maintain.
echo "ERROR: Merge conflicts detected. Manual intervention required:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit 5e12995.
- name: Config Git | ||
run: | | ||
git config --local user.email svc-api-experience-integrations-escalation@mongodb.com | ||
git config --local user.name svc-apix-bot | ||
- name: Rebase branch with master | ||
id: rebase-check | ||
- name: Merge branch with master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be cleaner to have this in a separate script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the moment I think it's better here because it also need to coordinate with the push step, so it's easier to see what's going on. If it get more complicated I agree to move it
echo "Pushing updated branch ${{ matrix.branch }} to remote" | ||
git push origin ${{ matrix.branch }} | ||
- name: Project check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be before the Push updated branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EspenAlbert as this is to do a merge without PR, I think it's better to avoid to to this manually as much as we can. So if the merge doesn't have conflicts, we push the changes, so later the checks can be fixed easily in a PR.
however if we don't push the changes, somebody has to manually fix it and merge, without a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I think the best way is to go ahead with this approach, and then later we can revert/change based on experience 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, thanks
Description
Update dev branches with merge. Don't use a PR as it will squash the commits and will cause the same conflicts to appear again and again.
It uses merge instead of rebase so history is not altered and changes can be reverted if necessary.
skip-docs-notification
label is not needed any more as we don't create a PR here.Link to any related issue(s): CLOUDP-329420
Type of change:
Required Checklist:
Further comments