Benefits for the developer of separating pull requests by logical change?

Question

I am currently responsible for reviewing one of my team mate's code for a front end web app. We have established a workflow which involves creating a feature branch in git for each feature we define. Say our app lets you scroll a list of items and click through to the item detail - then the kind of features we have defined so far are 'item list functionality', 'item list style', 'item detail functionality', 'item detail style'. Once the feature is complete, I have asked my team mate to submit a pull request and then we will work through the review process before the change is accepted.

For me, the benefits of separating pull requests into logical changes are that we can ensure all the code is focused and not redundant, that it is easy to review and we can hope to prevent tight coupling of components. My team mate sees this workflow as a hinderance to progress because he finds it very difficult to stick to this separation between feature branches.

I have a number of questions...

Are we setting the scope of our features too narrow?

At the moment most of the benefits of keeping pull requests focused are benefits for the person reviewing the code. Are there benefits for my team mate if he separates changes like this?

Answer 1

Are we setting the scope of our features too narrow?

I think features should be a useful, testable and releasable modification to the system. If you're getting smaller than that I think it may be too small.

At the moment most of the benefits of keeping pull requests focused are benefits for the person reviewing the code. Are there benefits for my team mate if he separates changes like this?

There are many benefits to having smaller discrete PRs. Here are some:

• Can release code more often to users (don't have to wait for a humongous PR with disparate parts), and can pick and choose features to release (e.g. we want to release feature A but not feature B. Can't do that if commits for A and B are all muddled together)
• Can test small increments. If your feature branch fails automated/manual testing it's easier to debug if the changes are fewer.
• Higher chance of PR getting merged. The fewer changes put for PR, the faster and the more self-explanatory the reviews will be. This is not only a benefit to the reviewer, but the reviewee as well.