3 ms·
I think these days the emphasis has changed a bit from "required reviews" (more senior dev or peer signs off on code) to "request review" (developer __wants__ t
by ritzaco 3y ago
I think these days the emphasis has changed a bit from "required reviews" (more senior dev or peer signs off on code) to "request review" (developer __wants__ the review, because it's a good learning experience and a way to share responsibility for the code (and any problems it might cause down the line).
Anecdotal, but I feel like I see more teams where that "Merge" button is available to most people, but in general doesn't get pressed until the code has at least one review, whereas before it was often only accessible to a select few, or a review was required before it activated.
- WorldMaker 3y agoI got into a bit of a fight over whether or not "Merge" should be an option on one's own PRs. "Trust" suggests that a developer should be able to review their own code changes, especially for small/trivial things or "expert" level things that no one else feels qualified to review and just leave open for weeks/months instead of even trying to review it (in turn often making it harder to merge without conflicts). There's a great value in "every change was peer reviewed by someone", of course. There's also a value in "not every change is significant and sometimes you can trust developers to know that". You can possibly install the fear of accountability if a bug happens in code that the reviewer was the developer until they want that learning opportunity of a code review. In my experience, most of the "expert-level" developers almost always would love any feedback anyone wanted to offer because their imposter syndrome tells them it is awful and not to sign off on it, but are also more likely to get blocked if their PRs don't move fast enough.