DEV Community

Discussion on: Pull Requests Are Slowing You Down

Collapse
 
outfrost profile image
Outfrost

Respectfully disagree.

  • The way we implement features and fixes matters. If it didn't, we wouldn't have a need for experienced engineers; all software would be built by bootcamp graduates working for barely above minimum wage. It also would've been predictable - something you yourself (correctly) argue is not true in the real world.
  • I get the feeling that you consider each developer's work as completely isolated, and a team's sole purpose being to nitpick code during review. That is of course not the case. A team works together on a product or component. Different developers write different portions of the system, and one developer's work ends up depending on another's. By going through code review before merging, you give your team the opportunity to alert you if your solution would run into issues that aren't immediately obvious. You also provide your team with higher quality code that they can depend on in their own work right away. In fact, if there are any dependencies between separate devs' work, doing code review early actually speeds up development, because you won't waste other people's time on going back and rewriting their code due to changes you make to resolve review issues.
  • From my professional experience, as well as studies and anecdotes from the industry: if you don't strictly require a process step before the work is delivered, people will start skipping that step, with excuses like "I'm working on something more important" and "I'll get to it later". It happens with cleanup, documentation, and, yes, review. You've already merged your changes, the integration tests passed, and 3 of your colleagues wrote and merged code that depends on yours. Are you going to review all that code now, and not only have your colleagues improve their individual contributions, but also rewrite them to work with a better interface design in your code, potentially spending days of everyone's work on it? Or are you going to shrug, say "whatever, it works fine", and move on to the next high priority thing?
  • Also from my experience, the earlier you review a changeset and the smaller it is, the easier it is to review. It's like continuous delivery, but on an implementation level - you get feedback about your solution earlier, which lets you adjust it more easily, and conflicts (whether git merge conflicts or logic conflicts) are far less likely.
  • Pair programming often isn't feasible, and it can put neurodivergent devs in an uncomfortable situation, resulting in stress, anxiety, and a decrease to productivity.
  • I would suggest to take a shift in perspective, and think about other problems that might be making code review seem unproductive. Why do you have to merge your changes and deploy in order to run integration tests? Why don't you run tests automatically on the pull request while you iterate or wait for review? Why doesn't every developer have their own local test environment? Not only is this slowing you down, but it also makes it more difficult to trace the causes of failures. "This fail looks weird, did I goof somewhere, or did Ash push something they're not done with yet?" Sure, you still probably have to do manual QA afterwards, but it helps use your QA time productively if all the changes that make it there are peer-reviewed and high quality. You don't want to waste your testers' work hours with a simple mistake that would've been caught in review.

So, to answer the questions :)

How much time are you spending waiting for pull request approvals every day?

None. I create a PR when I'm done iterating and testing, and I move on to the next task until changes are requested or approval is given.

How confident are you that integration tests will pass when you open a pull request?

They should've already passed in your dev environment, or you should see it in the PR itself, before anything is merged.

How many times has code that doesn’t work as expected been approved?

Probably quite a few! Code review isn't meant to catch the same issues as a test suite or QA session. Tests should verify that the code works as expected. Code review should give you better confidence that the code is understandable, maintainable, extensible, debuggable, secure, and won't run into problems difficult or impossible to catch in a test suite - in other words, everything other than "does it technically work".