Search code examples
gitversion-controlgit-flow

How to handle PRs when using Gitflow?

How many PR reviews should I be doing when running Gitflow? I have a feature branch with quite a few changes on it. I've PR'd it with reviews and merged into the develop branch. There may be like one or two small changes left before the release. When I go to merge the release code into the main branch, should I be doing another full PR review. Especially since most of the changes are going to be the same as the stuff I reviewed in the PR from feature to develop, it seems like a waste of time, but also with my current understanding, if I don't do reviews on the second PR from release to main, some things could slip through the cracks. How do you handle this redundancy or is there some sort of industry standard method for this? Thanks!

One thought is maybe PRs and their reviews are just required going from feature to develop?

Solution

  • When I go to merge the release code into the main branch, should I be doing another full PR review?

    No. In fact, if you can, consider automating those "Git Flow merges"1. If you manage to automate them, then you will only need to have manual intervention if there are conflicts. In properly structured Git Flow, it's not possible to have conflicts when merging release or hotfix into main, but it is possible to have conflicts when merging release or main into develop.

    One thought is maybe PRs and their reviews are just required going from feature to develop?

    That's correct, but note you also need PRs with full code reviews for all feature branches into any shared branch, which includes develop, release, and hotfix.

    General Git Flow Tips:

    1. After merging release into main, there are some (minor) advantages2 to merging main into develop instead of release into develop. Try it, you may like it.
    2. When merging feature branches into a shared branch, you can use a variety of merge strategies, such as regular merge, squash merge, and my personal favorite, semi-linear merge or rebase merge. However, when merging a Git Flow shared branch into another shared branch, you must use a regular merge. In strict Git Flow you should also use --no-ff for these regular merges which is necessary when merging release or hotfix into main.
    3. Consider protecting all Git Flow shared branches so that you have no choice but to do a code review in order to merge any feature code in. If you wish to automate the Git Flow merges, do so in a way such that the automation job is able to bypass the PR requirement.
    4. If an automated Git Flow merge fails (presumably due to conflicts), you can do the merge yourself, by creating a new temp branch from the target branch, merging in the source branch, resolving the conflicts, and then PR'ing that temp branch into the target branch. With this PR the only thing a code reviewer needs to look at is perhaps a sanity check of the included files, and then the merge conflict resolutions. Regardless of how you normally complete your feature branch PRs, this kind of PR has only 2 options: regular merge with either no-fast-forward, or, perhaps surprisingly, this would be the one scenario where a fast-forward merge would be allowed. The reason is that you already created the merge commit in your PR, and the resulting graph is the same as if the automated merge worked without conflicts. If you decide to go with fast-forward here, make 100% that you merged the source branch into the target branch instead of the other way around. If you aren't sure, then use --no-ff when completing this PR as well. It's better to have an extra unnecessary merge commit than to accidentally flip the first-parent history.

    1 "Git Flow merges" refers to any merge of one shared branch into another as required by Git Flow. Examples include merging release into master, release into develop, master into develop, hotfix into master, hotfix into release, etc.

    2 After merging release into main, some advantages of merging main down to develop instead of release down to develop, are: 1.) You don't build up a bunch of merge commits sitting all alone on main from each release. If they build up, when you eventually have a hotfix, all of those lonely merge commits get brought down to develop all in one shot, which looks kind of strange when you're looking at what commits are new with that merge. 2.) Before merging release into main, it may be useful to do a sanity check to confirm that release is fully ahead of main, because if it isn't, then it's probably because there was a hotfix that someone forgot to merge down into release and it's about to get blown away. One of the easiest ways to confirm you don't have this problem is to check that the tip of main is reachable by the release branch.