2

In my software engineering career I have never worked with pull requests. Probably because I only ever worked in relatively small teams (5-16 people) and only on projects that were structured well enough and had a small enough code base, which made it quite easy to keep track of all the changes other developers are making to it. While in those teams we've never used pull requests as a version control feature, we still had a process in place that required developers to first submit their feature branch to other developers for code review before it could be published. These feature branches when submitted for code review were "merge ready". That means that as soon as the person reviewing the code gave the green light, the branch could (and should) be published to master without any further changes to it. As you can already probably imagine, there was quite a lot of code to review for each branch. Depending on whether it was a branch for a bug fix or a full feature, amount of code that needed to be reviewed varied between as little as couple of lines and as many as 3-5k lines or even more.

While I do understand the concept of pull requests and what problems they are meant to solve, I am having hard time figuring out how pull requests can be incorporated into a software development process without it being too disruptive or a waste of time.

I always read on the internet how developers are complaining about their colleagues asking them to review enormous pull requests and how you as pull request submitter should always make sure that your pull request is easy to review.

I was thinking that one could create multiple pull requests during the development of a big feature. That would mean tho that you are pushing code into production that does not currently serve any real purpose and is very likely subject to change before the feature is completely implemented. Or you could submit a pull request without actually merging the current state of the branch into production. Kind of like a request to your colleagues "Hey, I'm developing this thing here. Can you check if I'm on the right track?" and only after the final pull requests the branch gets merged. But in that case it would mean that you are possibly wasting your colleague's time by letting them review code that has a very high chance to be changed before the feature is completely implemented and "publish ready". Another option would be to break down the feature into many smaller features. This would require quite a lot of planning to work and also has the negative side effect that you are publishing code that you will only need in the future, if at all.

How do those of you who have pull requests as part of their development process use them and how can one incorporate them into their development process without it disrupting people's work or wasting time?

1 Answer 1

5

A pull request is the same as having a code review when code is "merge ready". Whenever you have a code review and merge, replace that with a pull request and you have successfully fit a pull request into your development process.

As far as sizing goes, I would say that a code review or a pull request of 3-5k lines or more is much too large. Smaller changes are much easier to review and understand and can be accomplished by appropriately slicing the definition of work and including small numbers (ideally one) change per pull request. If you're using git, rebasing and editing your history with good commit summaries and messages can make it much easier to observe the series of changes and understand it step by step.

If your change is large, interim or work-in-progress pull requests can be helpful to get early and continuous feedback from the other developers. Having environments to put the running code can also be useful for feedback from product managers, user experience designers, and manual testers.

I've also found it helpful to have have automated tests and ensure that including tests and requiring a passing test suite is helpful before starting the review. Running static analysis, including style/formatting, and making sure that findings are addressed can also help make sure that people are reviewing good code.

2
  • I would say that thinking about pull requests as a benefit to code quality rather than "disrupting people's work or wasting time" is an important first step. They may add some time up front, but (at least in my experience), as long as the changes are appropriately sized, the benefits (catching defects early, sharing knowledge, reducing duplicated effort) far outweigh the time spent.
    – Jon E
    Commented Sep 21, 2020 at 18:10
  • @JonE If you already do code reviews prior to merging, then a pull request should be equivalent in all respects. As far as benefits, it depends on how you use the review time. You could use them to catch defects, or you could use them as an educational opportunity. Again, I'd probably recommend just using PRs exactly like you use code reviews as a first step and then figure out how to optimize the process or introduce changes.
    – Thomas Owens
    Commented Sep 21, 2020 at 19:19

Your Answer

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

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