15

Bob clones the project and creates a local branch A from master.

Bob adds some useful helper classes cleaning/refactoring unit tests setups and clean all exising tests thanks to them.

Bob commits, pushes to remote server, then creates a pull request in order to get a code review of this refactoring from John.

John, leading the project, is busy for a week so can't review it immediately.

After asking for code review, Bob wants to write some brand new test files and set of classes ending up with another separated pull request since it is considered as working on a new feature.
Obviously, Bob wants to use his new helpers for those test files.

Which strategy to adopt:

  • For those new unit tests, Bob should create a B branch derived from master and not A since A has not being reviewed yet. Drawback is that he can't use his unit test helpers yet since not existing in B.

  • Bob should wait for the code review of the first pull request, merge to master, and then derive B from master. During this time, he should focus on other works that do not depend on his previous pull request.

  • Bob should derived Bfrom A and use those helpers, taking the risk that A won't be accepted after review. Obviously leading to a rejection of B.

  • John should shake his ass and as a good leader, should review the first pull request in a short time so that Bob can chain.

What is a good practice to handle dependencies between multiple pull requests in series?

Mik378
  • 21,881
  • 15
  • 82
  • 180
  • 1
    Great question. I'd go with option 3. *Bob should derive B from A and use those helpers, taking the risk that A won't be accepted after review. Obviously leading to a rejection of B.* Bob can always go back and fix what's wrong with branch A. – MMalke Mar 02 '21 at 21:05

1 Answers1

3

It’s unnecessary to depend and wait your current pull request(PR) on previous ones. For your situation, Bob wants to continue develop/test something based on branch A after a processing PR (issued and not approved yet). He just need to develop the code on branch A, and then commit & push to remote. The PR which merge branch A into master will automatically contains Bob’s second time changes.

So for the multiple PR situation, if you want to update the branch which has already in a pull request, you just need to commit & push changes, the previous PR can update automatically. If you want to update the branch which is not contains in a processing PR, you need to commit & push changes and then create a new PR, and it has no effect on the previous processing PR.

If you just want to add some tiny changes or the feature itself, you should make changes on branch A and use the processing PR. If you need to develop new features, you should make changes on a new feature branch and create a new PR.

For Bob’s condition, developing new feature and need to use helpers on branch A, so he should derive branch B from A even A is not reviewed yet. Because developers need to consider how to develop new function and it’s not efficient to develop new feature until the previous PR is approved. Or if your team really need to use this way, you can still derive B from A and rebase branch B for your needs.

Marina Liu
  • 36,876
  • 5
  • 61
  • 74
  • 1
    I fully agree and I would work like this with MY team. The "hic" is that I had this discussion 24 hours ago with a tech lead near to me and he told me: "No... if the PR gets updated, the number of unrelated files (mix of "features") will grow and it is likely to be very difficult for me to perform ONE code review for the whole at once (too many changed/added files at once)... so I suggest them to write a second and completely independent pull request". It might be a good practice that's why I'm a little bit confused and wrote this post. – Mik378 Feb 01 '17 at 04:28
  • I updated my OP with some details about the content of the second pull request. – Mik378 Feb 01 '17 at 06:28
  • 1
    Yes, you can derive a new branch from branch which you need, and create a new PR. And I updated my answer. – Marina Liu Feb 01 '17 at 07:17
  • 1
    What if the review of A is not accepted? This would lead to a kind of new PR from branch C like "Remove helpers", this one based on branch B? – Mik378 Feb 01 '17 at 07:33
  • Even the A is not accepted, but it not affect B and C, because they are different. there has new features for B and C, so you can continue the PR for B and C. – Marina Liu Feb 01 '17 at 08:31