Commit any conflicts during v1 backport to simplify release process #1032
Conversation
The process of creating the v1 release can run into merge conflicts. We commit the unresolved conflicts so a maintainer can easily resolve them (vs erroring and requiring maintainers to reconstruct the release manually).
There was a problem hiding this comment.
IIRC, the automatically generated PR will be in draft mode and not run any jobs. If a user marks the PR as ready-for-review, checks will run and (presumably) some checks will fail due to merge conflicts.
I guess, maybe that's not true if the merge conflict is in a file that we don't compile or run (eg- CHANGELOG.md). Is this a likely possibility that one of the files with merge conflicts through this backporting process will not cause a workflow failure?
In general, I'd like to avoid having the only check against merge conflicts be in the form of the checklist in the PR, since PR checklists are easy to ignore. Though, I'm willing to believe that this will only happen in a handful of files that we execute anyway and would already cause some sort of failure.
|
That's a fair comment. It shouldn't be likely, but it's definitely a possibility that a file like It might also be reasonable to require that any releases containing merge conflicts are reviewed by someone other than the person who fixed the merge conflict. |
|
Yep. That's one solution I had in mind. It doesn't need to be implemented now. I'm not even sure if it's necessary. If the only merge conflicts will be in workflows that are run on every PR, then it's not necessary. |
|
I agree with what Andrew said. May I suggest that you add the |
|
Thinking about it again, I wonder whether it's more useful to sacrifice clean git history in favour of making a separate commit to fix the merge conflicts. This would make it clear what conflicts we needed to fix manually and aid PR review, since maintainers should just be able to review the commit fixing the conflicts instead of the whole merge commit. What do you think? |
|
I added a workflow to check for git conflict markers. Here's an example of it failing: https://github.com/github/codeql-action/runs/6029239038?check_suite_focus=true. |
henrymercer
force-pushed
the
henrymercer/handle-merge-conflicts-in-releases
branch
from
30b3365 to
4fd6d94
Compare
This check is primarily intended to validate that any merge conflicts in the v2 -> v1 backport PR are fixed before the PR is merged.
henrymercer
force-pushed
the
henrymercer/handle-merge-conflicts-in-releases
branch
from
4fd6d94 to
5b5ed44
Compare
In my mind, having a separate conflict fix commit in the PR would not affect the v2-to-v1 merge automation because the v1 commit that is being reverted by automation is the last PR merge commit, so that revert should cover all commits that are in that PR. Is that correct? If so, then I think having a separate merge conflict fix PR would not be a big deal. |
…ge commit This gives us slightly messier git history, but more importantly makes reviewing substantially easier.
That's correct, yes. Let's do separate fix commits. |
Merge conflicts can occur during the v2 -> v1 backport due to differences between the v2 and v1 releases (e.g. the Node version). Currently the process for dealing with these is fixing the v2 -> v1 backport branch manually and opening the PR yourself, however this is a bit of a pain.
This PR modifies the process: automation now checks in any unresolved conflicts and alert maintainers in the PR description of which files contain conflicts and need fixing. Then all maintainers need to do is push a commit that fixes the conflicts.
One method of doing this that maintains clean Git history is fixing up the merge commit withWe will favour adding a new commit to resolve the merge conflicts instead of amending the merge commit — while the latter leads to clearer git history, the former significantly improves the ease of PR review. When a maintainer needs to resolve conflicts as part of the release process, per standard practice, they should request review of their changes from another maintainer before merging the PR and continuing with the release.git commit --fixup <merge commit SHA>andgit rebase -i --autosquash --rebase-merges releases/v1.Example checklist for a backport PR with conflicts (this is backlinked below for maintainers):
Merge / deployment checklist