1

Nobody wants to review a large PR from one go. The issue is to split a (relatively large) feature into multiple smaller PRs. The rules of the game are

  • This is a single feature, which cannot be meaningfully split into multiple standalone features
  • The whole purpose is to make the reviewer's job easier, by making "unit" pull requests. This means that there is more opportunity to describe each unit and better communication with the reviewer. Also, helps focusing the review.
  • While units can be covered by unit tests, the meaningful tests require the full functionality.

For an example, let's assume that there is a feature which would be implemented as

niceFeature() {
    stepA();
    stepB();
    stepC();
}

Each step could be reviewed and unit tested independently. So, there could be four branches, each of them being based on the previous one

master 
      \-> addStepA 
                  \->  addStepB 
                               \->  addStepC 
                                            \-> addNiceFeature

The problem is that if during the review of addStepA, there are some changes to be made, this would require rebasing everything. Which would be fine, but there could be many (possibly conflicting) rebasings with the whole goal of testing the reviewer's suggestions for addStepA and their impact on the niceFeature. Is there an easier workflow?

1
  • Not a perfect solution, but reviewers can review a PR 1 commit at a time. Though in my experience, bad quality commits ("fixit", "tmp", "addressing PR feedback") have trained people to ignore commits in PRs entirely).
    – Alexander
    May 5, 2022 at 22:52

2 Answers 2

3

Look at all of the development work required for that feature and split it into multiple discrete tasks. Each task becomes its own branch and its own code review.

How you handle version control is a separate issue. Avoid deeply nested branches if possible. This advice holds true for any version control system. Nested branches imply dependencies, which complicates planning and execution of work. How you handle version control is dependent on development methodology.

For instance, if your team practices trunk-based development each task can be merged into your main branch. New code can be conditionally executed using feature toggles. This implies creating one branch per task and merging each task branch into the main branch:

master ------------M------------M------------M
       \-> step A / \-> step B / \-> step C /

Each M represents a merge and pull request.

Alternately (for instance when using Git Flow), create a topic branch for the feature. Each task branch gets created off the topic branch. Pull requests would merge each task branch into the topic branch for the feature. Finally after all tasks are complete, merge the feature branch into your main branch (which should be another pull request).

master -------------------------------------------------M
     \-> feature ----------M------------M------------M-/
               \-> step A / \-> step B / \-> step C /

Either approach gives you the flexibility to decide which tasks depend on each other, and which tasks can be worked on concurrently. Whether doing trunk-based development or using topic branches, you will have an integration task to verify the whole feature is working as intended before shipping this off to QA.

2

Show code to your teammates earlier and more often.

If you have already done addStepA, addStepB, addStepC, addNiceFeature, no amount of Git finessing will help you when your teammates point out that B is not needed, and C should instead be Z.

Improve your team’s workflow so that code review is always top priority and thus you are not afraid of creating small pull requests early. Align with the team on the general approach up-front. Split tasks into smaller ones in your task manager.

By the time you’ll feel you’re aligned well with your team, rebasing a few branches won’t be a problem, as your team will be mostly rubber-stamping the PRs they have already de facto agreed on.

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.