Skip to content
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

PHPC-2375: Manually merge up version bump commits #1586

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jun 14, 2024

PHPC-2375

This PR adds logic to automatically merge up changes to the next release branch. This is necessary as the pull request for the merge-up action will always have a conflict due to the version already having changed in the next branch.

If the branch detection logic finds a newer branch to merge to (which it always should as we're releasing from version branches), it runs a git merge --strategy=ours in this next branch, then pushes those changes before pushing the release branch. By pushing the next branch before the release branch, the merge-up action discovers that there are no commits to be merged from the release branch into the next branch and will not create a merge-up pull request.

Note that if the release branch contains changes that have not been merged up yet, those changes will be marked as merged without applying them. I can add logic to detect this and prevent a release, but this case shouldn't come up very often as we're quite diligent about merging the merge-up pull requests so I've decided against it for the time being.

@alcaeus alcaeus requested a review from jmikola June 14, 2024 07:09
@alcaeus alcaeus self-assigned this Jun 14, 2024
@alcaeus alcaeus force-pushed the manually-merge-version-bump branch from 23d0d0a to 238bb74 Compare June 14, 2024 07:10
@@ -62,6 +62,7 @@ jobs:
app_id: ${{ vars.APP_ID }}
private_key: ${{ secrets.APP_PRIVATE_KEY }}
submodules: true
fetch-depth: 0
Copy link
Member

Choose a reason for hiding this comment

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

Noted that this fetches the full history (secure-checkout/action.yml).


- name: "Determine branch to merge up to"
id: get-next-branch
uses: alcaeus/automatic-merge-up-action/get-next-branch@main
Copy link
Member

Choose a reason for hiding this comment

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

Do you intend to merge this to mongodb-labs/drivers-github-tools at some point? If so, should there be a TODO?

Edit: I didn't realize alcaeus/automatic-merge-up-action existed already. Looks pretty involved (beyond the small actions in the DBX repo), so I understand if you want to leave this as-is. I think you should probably target a tag instead of @main, though.


Side note: WTF is dist/get-next-branch/index.js 20 KLOC? I assume you didn't write that yourself.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll keep the action separate for the time being. Once I've confirmed this works I'll tag the current state at v1 and update all of our repositories accordingly.

As for the 20 KLOC, all of the dependencies are compiled into a single file so that running the action from a workflow doesn't require installing them manually. The main dependencies are the actions toolkit that makes dealing with inputs, outputs, and the GitHub API easier.

- name: "Push tag and release branch"
run: |
git push origin ${RELEASE_BRANCH}
git push origin ${{ inputs.version }}
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, would git push origin tag ${{ inputs.version }} be less ambiguous? I picked that up while reading the git-push docs and https://stackoverflow.com/q/5195859

I also noted the advice in the SO thread to avoid using --tags, which we historically did (see: RELEASING.md). It appears that tag-version/action.yml is still doing that, so perhaps you'd like to revisit that at some point so it only explicitly pushes one tag.

Copy link
Member Author

Choose a reason for hiding this comment

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

TIL tag <name> is a thing. Updated to ensure what we're pushing is a tag ref.

Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

OK with this as-is. You can decide to implement any feedback.

@alcaeus alcaeus enabled auto-merge (squash) June 17, 2024 06:49
@alcaeus alcaeus merged commit c72bde6 into mongodb:v1.19 Jun 17, 2024
53 checks passed
@alcaeus alcaeus changed the title Manually merge up version bump commits PHPC-2375: Manually merge up version bump commits Jun 20, 2024
@alcaeus alcaeus deleted the manually-merge-version-bump branch June 20, 2024 09:17
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