• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Tim Cooke
  • Jeanne Boyarsky
  • Bear Bibeault
Sheriffs:
  • Knute Snortum
  • paul wheaton
  • Devaka Cooray
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Ron McLeod
  • Piet Souris
  • Ganesh Patekar
Bartenders:
  • Tim Holloway
  • Carey Brown
  • salvin francis

An efficient Pull Request and code review workflow

 
Marshal
Posts: 4662
301
IntelliJ IDE Clojure Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a code workflow dilemma to resolve.

I'm a developer on a small team. Our current workflow for pushing code changes is to branch from the main 'dev' branch, do your work, then create a Pull Request for approval back into 'dev'. My development cadence us usually quite fast, I'll write tests, implement features, fix issues, refactor unpleasantness, all in a relatively short space of time, which is great but at the end of that I am inflicting an unmanageably large diff in the resultant PR that gets sent out for my team members to review. Because of this I usually end up having to sit down with them and walk through everything I've done. At the last review it was suggested / requested that I keep my PRs smaller to be easier to review. This is a fair enough comment.

The problem I have now is that I don't know how to adapt my workflow in order to produce smaller PRs and maintain a reasonable cadence of development. For example, my current bit of work started out with writing a new covering test for some existing functionality, and resolving a bug that I happened to find along the way. That seemed like a nice sized chunk for a PR code review. All is good. But..... it formed the basis for the next bit of work but the first PR has not been approved yet so I can't submit the second PR without it including the first also.

I hope this is making sense.

My question is: Have you any suggestions from experience about Pull Request size, frequency, scope, or anything else that works for you to maintain good development speed while making it easy for your colleagues to review your output?
 
Saloon Keeper
Posts: 10495
224
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
How small is small enough is very subjective. It's a balance you have to find out by communicating with your coworkers. I can give you a suggestion for how to make your PRs smaller though, without losing too much velocity:

After you've written a bite size chunk of code that you think you coworkers will be happy to review, create a branch from the commit up to that point. Make a PR for it, and keep working. Keep doing this, and each time you've finished another chunk, make a PR for it that merges the new chunk into the branch containing the old chunk. That way, each PR will only display the changes for the next chunk.

Finally, when all chunks are done and reviewed, you merge all your PRs into the feature branch. This branch will then contain the combined changes you've made in relation to the dev branch. This changeset will be the same as the big PR that your coworkers would have had a harder time reviewing, except this time they've already reviewed all the code, and they just have to sign off on merging it into the dev branch. Let's look at an example:

You're working on a feature and you've made a branch "features/discombobulator" for it. You immediately make another branch "features/discombobulator-p1" and start work. When you're happy with the changes and you think the amount is good for your coworkers to review, you create a new branch "features/discombobulator-p2" off of it, and you make a pull request to merge "features/discombobulator-p1" into "features/discombobulator". When you have another set of changes ready, you make a pull request to merge "features/discombobulator-p2" into "features/discombobulator-p1", and you continue work on "features/discombobulator-p3".

When you're done, you'll have three pull requests, each merging changes into the previous branch. Your coworkers can review them all separately. When you've made updates to each of your branches and your coworkers are happy, merge all the branches in reverse: "p3" into "p2", "p2" into "p1", "p1" into "features/discombobulator". Finally, you create a pull request to merge "features/discombobulator" into the dev branch. This will give your coworkers a final opportunity to review the entire feature, but it will go faster for them because they've already seen most of the code, and there won't be as much to fix because most mistakes will already have been fixed in the previous pull requests.
 
Tim Cooke
Marshal
Posts: 4662
301
IntelliJ IDE Clojure Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That's an interesting approach Stephan, thanks. I'll have to discuss with the team and see what they think of it.
 
Would you turn that thing down? I'm controlling a mind here! Look ... look at the tiny ad ...
professionally read, modify and write PDF files from Java
https://products.aspose.com/pdf/java
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!