If "usually" PRs result in code style or architecture discussions, I'd suggest you have a team problem, not a process one. Start with the humans.
I also object to spotting bugs as an example of misuse. I regularly point out possible bugs in PRs - even ones with unit tests. It often turns out to not be a bug, but does result in new test cases or a comment added.
Not mentioned is one of the other big benefits of PRs (which applies no matter if you pair or are a solo project): it enforces some good discipline.
If your PR description involves several unrelated things, it's too big and should be done in smaller chunks. I find it gives me a really good chance to scrutinize my own code: things like finding TODOs I missed, bad variable names (which maybe made sense in the first iteration but not in the final state), or bits that need some extra documentation.
I don't pair a lot, but when we do (on my current team), we still use PRs, with both as reviewers, as well as getting someone else to come in. The 3rd person is always going to be better at spotting unclear code then the people who've been neck deep in it for a day or 5.
I twitched when the author's FIRST point was about code style; in this day and age that should not be up for debate anymore, run it through a formatter, change the formatter's rules if you don't like it, and have your CI reject it if the user did not run the formatter before anyone has a look at it. Code style is a distraction that a reviewer should not have to spend their focus on, not when the REAL problem is yet to be reviewed.
Yeah, I am totally with you on this one. Code style shouldn't be up for debate anymore. Nowadays we have plenty of linters and code style fixers that automates the whole code style of the project. That wasn't even the point I was trying to share on this article. PR and PP are on another level, focus on sharing knowledge rather than "code style", for sure :)
> If "usually" PRs result in code style or architecture discussions, I'd suggest you have a team problem, not a process one. Start with the humans.
+1
> Not mentioned is one of the other big benefits of PRs (which applies no matter if you pair or are a solo project): it enforces some good discipline.
I found that pairing and frequent rotations result in a much more uniform adoption of good coding practices, following working agreements, etc... It just happens much more frictionlessly since people learn faster by doing. Again, frequent rotations and ensuring that people mix as much as possible is key. (one company I worked for adopted a so called "promiscuity table" to keep track of that)
Also, to quote someone more experienced than me "I've never seen a +300LOC PR that didn't look good!"
Counterpoint: I've reviewed multi-thousand line PRs (with dozens of commits) ... those took me days of 8 hour review, with multiple issues found. (To be fair, when I first skimmed it, it looked great to me, but a deeper dive found lots of things that could be improved.) These kinds of large PRs was how the gentleman I worked with did pairing and it worked out pretty well (surprisingly, don't try this at home).
I suspect this is the exception that proves the rule, not really an alternative rule.
I added linters and prettier. We don't have this discussion.
Anything that isn't important I prefix with nitpick and my team knows to ignore if they want to.
Actual feedback should be based on functionality.
Having said that, sometimes pair programming helps. It sucks when you're remote. And it really sucks when you have tiny tiny desks as is the new startup trend.
I honestly miss nice big cubicles. Say what you will, they served a great purpose. Every eng I knew with a nice big cubicle had a 2nd keyboard for the other person that sat in.
I also object to spotting bugs as an example of misuse. I regularly point out possible bugs in PRs - even ones with unit tests. It often turns out to not be a bug, but does result in new test cases or a comment added.
Not mentioned is one of the other big benefits of PRs (which applies no matter if you pair or are a solo project): it enforces some good discipline.
If your PR description involves several unrelated things, it's too big and should be done in smaller chunks. I find it gives me a really good chance to scrutinize my own code: things like finding TODOs I missed, bad variable names (which maybe made sense in the first iteration but not in the final state), or bits that need some extra documentation.
I don't pair a lot, but when we do (on my current team), we still use PRs, with both as reviewers, as well as getting someone else to come in. The 3rd person is always going to be better at spotting unclear code then the people who've been neck deep in it for a day or 5.