[ckdarby]

I'm writing a book, "Bytes Of Wisdom: From Start To Merged" that focuses around code reviews.

I agree that larger PRs can increase the latency of getting feedback and getting quality feedback. I also have another view.

Most code reviews struggle not from the reviewing part and shockingly not the authoring part, but the laying groundwork part. The laying the groundwork part is before any hands to the keyboard to write actual implementation. This part focuses on explaining what is trying to be solved and for what problem. It is bringing everyone who would like to participate in the conversation up to speed on terminology, background, understanding of past decisions and the different proposals looked at, ruled out, etc.

I've seen too many PRs ask for reviews where the reviewer had to read the description half a dozen times and spend an additional hour understanding other systems that interact with the diff to be able to even attempt to discuss if the implementation solution is even the "correct" implementation that should be tackled for the problem at hand.

IMO, there are four key parts to better quality PRs, laying the groundwork, authoring the technical implementation, reviewing and feedback.

[mdgrech23]

Feel like this is complicated by the fact that a lot of the orgs I worked followed Agile and had cards w/ the work divided up in weird ways. A lot of times I was doing shit just b/c the card said so, if I had more autonomy I would of been doing a totally different subsection of work.

[mvdtnz]

I agree with you for significant changes. But most day to day programmers are working on large and/or mature codebases, and I would think that this level of groundwork should not be required for a typical change. Indeed I would hope I don't need to bring multiple stakeholders along on the journey in order to add a couple of columns to a report or upgrade an API schema version.

[ckdarby]

I understand and it is a bit of a struggle I'm facing in the writing to have the reader believe that a large part of what is holding them back is this very thing. A lot of my technical audience has the sense of, I should just be able to make my change and shouldn't need multiple eyes along the journey for feedback.

I had an experience very similar to your, "just add a couple of columns". That experience which I'll explain was the final straw to getting me to start drafting the book.

An individual at an enterprise company made a PR to change the code from handling a single item to a list of items. Of course this showed promising performance because it reduced a bunch of the overhead operations that used to be O(N) down to some sub N amount.

It took me about 3 hours to fully understand the surrounding systems. At that point I did not review the implementation, BUT instead the problem. A system was falling behind and timing out. This implementation did help fix the issue by batching API calls. While I as the reviewer did the "laying groundwork" portion I realized that a lot of the API calls themselves did not need to be triggered in the first place.

Looking purely at the implementation for review was a LGTM (Looks Good To Me) kind of review because I could not weigh out the solutions to the problem without the information on the systems to understand where the problem could be.

[gavmor]

Well that sounds like a good read! I've been privileged to spend my 10+ years writing software among pair-programmers, and have never been made to suffer from this "code review" ritual. Maybe it's not so bad? I'd love to read a book about it before my luck runs out and somebody foists it upon me. Just to be prepared, you know.

Seems to me like it just can't work. Show a programmer a 5 line program, as the adage goes, and get 5 critiques. Show her a 500 line program, and get a thumbs up; "looks good!"

Really curious to know what keeps professionals from resorting to that timeless adage. Besides sheer chutzpah, how do folks muster the gumption to tell their peers what should have been tackled? Or, do I have my hat on backwards?

[mvdtnz]

Code review is not a "ritual". Our code is our output, and it is paid work. Most professions have some level of review. Go speak to a chartered accountant or an engineer and complain about the "ritual" of peer review and see how far you get.

We are professionals and it's about time we started acting like it.

[gavmor]

Of course code review is a ritual. All professionalism is. We establish norms and rites the performance of which increases shared understanding and predictable outcomes. I am not sure I understand what malign connotation this word carries for you.