3

I prefer to create linear git history, which can be fast-forward merged. This was a technical requirement at my previous job, which used BitBucket and required all PRs to be fast-forward-able. We had to always manually rebase onto latest master. A new company I recently joined, using AzureDevOps, likes to have small PRs all targeting master/main (AzureDevOps has nicer merge options like "Semi-linear Merge" (Rebase+Fast-Forward+Empty Merge commit) and "Rebase+Fast-forward".

However, some changes will build on top of others. So sometimes I need to create stacked PRs.

Git/PR branching strategy aside...

I have 9 outstanding PRs. They are mostly very tiny. Regardless, I have a lot of code that is not merged to master/main. Now I'm being strongly encouraged to work on some new feature, but my manager wants me to branch off of master/main, instead of using the latest infrastructure upgrades I've completed. I'd rather not build on top of old infrastructure because my PRs haven't been approved over the holidays.

The more general question here, is:

If you are on a "team" of 1, sometimes 2, developers, and your PRs are not getting reviewed at all, should you be branching off of master/main?

There are other developers not working in the codebase that often help with code review (I find this very odd). I am a front-end dev working in our front-end repo, but there are some (very smart) backend devs that have helped with code review for front-end. They don't necessarily have any mechanism where they prioritize code review for the front-end. Our backend devs have extremely high standards, and have consequently (somewhat) been throwing their hands up because PRs created by very-junior "overseas" developers are extremely large and not easy to review.

Long term I don't intend to stay on a team of 1, where we insist on code review.. but there are no developers that are impacted by code not being merged to master/main.

My original google search: software engineering stackexchange if PRs aren't being reviewed should you keep branching off of master/main

Related: Dealing with a large pull request

Improve this question
4
  • My sort of interim strategy is to maybe build on top of new infra, and then if I'm forced to use old infra, really do bare minimal steps to get it running (ignore errors/hack/patch/etc). Since all those errors are only due to using old infra.
    – Devin Rhode
    Jan 5, 2022 at 14:49
  • Are you permitted to mark your own code as reviewed?
    – Caleth
    Jan 5, 2022 at 14:57
  • The merge policies in Azure don't permit that. The team wants code review to happen.
    – Devin Rhode
    Jan 5, 2022 at 15:07
  • 2
    "my manager wants me to branch off of master/main" - talk to them about why, and what risks that entails. For example: you don't get the infrastructure upgrades that way, whatever that means to you, but also you could then merge the new changes before the upgrades are approved.
    – jonrsharpe
    Jan 5, 2022 at 16:13

4 Answers 4

Reset to default
11

I have a lot of code that is not merged to master/main

This is a "management smell". It signifies that continuous integration has not been so continuous. Sometimes it's unavoidable - there's been recent discussion of a Linux mega-refactoring that's over 2000 changes - but it's something that should be prevented.

Unfortunately there's no way round this other than to start making noise. Report the number of outstanding code reviews you're waiting for in every standup, or otherwise to your manager/team every day until the situation improves. Ask specific individuals if they can do a code review for you. Drop meetings into people's calendars to reserve their time to do code reviews. If you're physically in an office together, walk up to people and ask them to do it.

This will irritate people, but so long as it's kept by the book and professional you can turn any pushback into more pressure on the review issue; maybe they'll change the way the team works, maybe they'll exempt you from having to do reviews.

Improve this answer
1
  • (thanks for the acceptance, but generally on stackoverflow one should wait about a day before accepting an answer in case better answers appear)
    – pjc50
    Jan 5, 2022 at 17:08
3

If you are on a "team" of 1, sometimes 2, developers, and your PRs are not getting reviewed at all, should you be branching off of master/main?

You should make it a practice to always start your new feature branch from master/main (or develop, if your team uses that) and it is even more important if you have a policy of doing code reviews on pull-requests.

The only reasons to deviate from that policy should be if the new feature actually depends on the old feature (cannot be implemented properly without having the older feature's code available), or if you would get a ton of merge conflicts otherwise (for example, if the older feature contains a large refactoring).

The main reason for always branching off from master/main/develop is that it gives you a lot more flexibility in the order in which stuff can be merged back after review. The problem with branching off from a feature branch is that either the review have to be looked at (and preferably initiated) in the same order that you created the branches to avoid that all changes pile up in one review (where the reviewer might also say "I approved this already in that other PR, why am I looking at it again").

By branching off from the branch you need the code to be merged back to, you ensure that the pull-request (and thus the review) will only contain the changes you made for that one feature.

And, as said, it gives flexibility. If you first implement a feature with some complicated logic in it, that takes real effort to review well, and follow that with a trivial change (like, repositioning a button), then the reviewer can choose to first look at the trivial change and postpone the other change till they have the time to really look at it. Some reviews might stay open a bit longer than others this way, but you will probably feel more progress being made than when all reviews stay lying around because they all contain that complex piece of logic.

Improve this answer
4
  • After branching feature/walls from the in-review feature/foundation work, it should be relatively easy to rebase feature/walls onto main branch when feature/foundation is approved, before submitting the pull-request. Then reviewers won't see the feature/foundation changes again. Just because you branched from feature/foundation doesn't oblige you to merge to that branch.
    – Toby Speight
    Jan 11, 2022 at 14:10
  • @TobySpeight, by doing that, you are forcing that the reviews are done in the same order as that the feature branches were created. If you branch of feature/ceremonial-first-brick-plaque from /feature/walls, then you cannot review feature/ceremonial-first-brick-plaque without reviewing feature/walls at the same time or before.
    – Bart van Ingen Schenau
    Jan 11, 2022 at 14:56
  • 2
    Pretty much, but that's inevitable if you have one feature that depends on another. I don't disagree that independent features should all come from their target branch (main), and I like to re-base to the head from time to time if it's a long-lived feature branch. I was only talking about the case in your second paragraph, when features are strongly dependent.
    – Toby Speight
    Jan 11, 2022 at 15:21
  • 1
    That said, if you can take seemingly-interdependent features and find ways to make them independent, that's really the way to go.
    – Toby Speight
    Jan 11, 2022 at 15:22
3

The number of pull requests created and pull requests approved is the same in the long term. As a consequence the people creating pull requests must also perform their fair share of approvals. As you noticed, delaying approvals creates pain.

As your number of unapproved pull requests grows, there is a task for you that grows in importance: Make your colleagues review your pull requests. Call them personally telling them that there are open pull request that need reviewing and they need to review them now. Highest priority is almost always tasks that stop colleagues from twiddling their thumbs.

So with nine open pull requests, you should most likely not do any branching, but you should spend your time prodding your colleagues to review them.

Improve this answer
0

When the infrastructure work is commissioned, it is incumbent on the “team”(which is apparently you alone) to dual implement each subsequent PR (over a finite window of time, perhaps long after the reworked infrastructure's PR approval-commit-release to support bug fixes in released versions) with & without the altered infrastructure. Then when a PR is either impossible or impractical to implement with the old infrastructure, everyone (most especially management) sees the value of the improved infrastructure. Bug fixes in released versions get their own PR along each affected release branch as well as a separate PR along main-branch/new-development, and in your case with & without the altered infrastructure.

You seem to be looking for an effort reduction. None exists once you altered the infrastructure then submitted that modification along that PR's branch. Dual development with & without the altered infrastructure is the expected norm until all PRs against the old infrastructure cease to exist.

TL;DR: Management is correct and you are wrong in this case because management needs the bug-fix PRs fixed on the old infrastructure if at all possible to mix in the fix on release branches too.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.