3 ms·
All of this should be hammered out in a set of guidelines around code review so that ego/feelings don't need to come into play. Have style guides, code review c
by harrisonjackson 4y ago
All of this should be hammered out in a set of guidelines around code review so that ego/feelings don't need to come into play. Have style guides, code review checklists, and best practices documented.
The actual words used should be determined by the goal of code review and the relationship between reviewers.
Anything that can possibly be automated by CI should be. Style guide and test coverage should automatically fail PRs before a human needs to add any "nit"
That means code review should be a teaching or question moment (between a senior and junior, newer and veteran) or a discussion between peers.
From a junior to a senior -> why did you extract this? why didn't you do it this way?
From a senior to a junior -> typically, I would extract this because X. Check our best practices doc for a longer explanation.
Between peers or senior/junior (maybe one is newer) -> it isn't in our coding best practices document, but normally we extract this because X
Between peers -> what do you think about extracting this because X? Should this be in the coding best practices document?
The last is important because ideally you're updating your coding guide so this same review doesn't need to be done 10 times. And, if you don't think it should be documented as a best practice then don't bring it up in a review.