When I did code reviews, and for any team I lead from a technical aspect for, my a review has three phases of confirmation:
Sure it seems like a lot, and yes it may slow down the merge process a bit. But in my experience taking a little longer to review code can help prevent issues from getting to production.
The holy grain though is a 100% automated process with confidence in the automation that when a change begins throwing errors the changes are rolled back, the errors logged, and the committing developer alerted. But that is a story for another time.
Adding to this I ask to use screencastify plugin and create gif to compare experience/features before and after changes.
Wow, I need to write that down. Great list
Great looking checklist!
Love this list!
A comment on the first point...
I don’t always stop if the tests aren’t passing , especially when reviewing less senior level engineers PRs. I’ve found that in a collaborative environment, a detailed review in spite of test failures often shows an obvious mistake that can quickly be pointed out, increasing the overall velocity of the initiative.
One possible addition...
Reviewing the test additions or updates first can often guide the review nicely, framing the changes in an easy to follow and logical way. Plus, if reviewing someone for the first time, it helps clarify the patterns and structures they use.
Really thorough answer. I agree with it all!
We're a place where coders share, stay up-to-date and grow their careers.
We strive for transparency and don't collect excess data.