5 ms·

BTW does anyone have a good guide, book, tutorial on doing code reviews? Everywhere I've been its kinda adhoc with no rules or guidelines. Often code reviews c

by rb808 8y ago

BTW does anyone have a good guide, book, tutorial on doing code reviews? Everywhere I've been its kinda adhoc with no rules or guidelines. Often code reviews come in after someone has put a few weeks of work into something and its too late to change how it was done. Or a review is passed in by someone in a hurry without actually critiquing it.

ydnaclementine 8y ago
my one suggestion is to make your code reviews have more questions than statements. so instead of: ‘forgot to check for null here’, do something like: ’do you think we can have a null issue here?’. in my mind, this word change makes the reviewee feel like they’re not just being told what to change, but actually have a sense of code ownership and control over their work
sauceop 8y ago
We had a senior engineer (who sadly moved on to a different company) periodically give a talk to new hires about good code review practices to set expectations and ground rules. Some of the key points: * Authors are expected optimize for readability of code, even if it takes longer to get the code in. * Reviewers should generally only start reviewing code if they are ready and willing to continue the review to completion (i.e. no drive-by reviews). Once they start reviewing they should try to minimize response time. * Focus the reviews on testing, correctness, readability * Consistent code style is important (see optimizing for readability) * Break code reviews into the smallest reviewable units. Often these are somewhat large because parts of the change don't make sense in isolation * The code author is responsible for writing code in a way that it convinces the reviewer that it is correct (see optimizing for readability) * You should approach it collaboratively - you're working together to get the work done and make the codebase as maintainable as possible * You should aim to leave each bit of the codebase that you touch in at least a good a state as it was previously

by rb808 8y ago

ydnaclementine 8y ago

my one suggestion is to make your code reviews have more questions than statements. so instead of: ‘forgot to check for null here’, do something like: ’do you think we can have a null issue here?’. in my mind, this word change makes the reviewee feel like they’re not just being told what to change, but actually have a sense of code ownership and control over their work

sauceop 8y ago

We had a senior engineer (who sadly moved on to a different company) periodically give a talk to new hires about good code review practices to set expectations and ground rules. Some of the key points: * Authors are expected optimize for readability of code, even if it takes longer to get the code in. * Reviewers should generally only start reviewing code if they are ready and willing to continue the review to completion (i.e. no drive-by reviews). Once they start reviewing they should try to minimize response time. * Focus the reviews on testing, correctness, readability * Consistent code style is important (see optimizing for readability) * Break code reviews into the smallest reviewable units. Often these are somewhat large because parts of the change don't make sense in isolation * The code author is responsible for writing code in a way that it convinces the reviewer that it is correct (see optimizing for readability) * You should approach it collaboratively - you're working together to get the work done and make the codebase as maintainable as possible * You should aim to leave each bit of the codebase that you touch in at least a good a state as it was previously