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.
Really thorough answer. I agree with it all!
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!
For frontend development, there seems to be an additional step that is needed.
Pull Requests should be accompanied by UI reviews. Because you can only see so much in code.
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.
Are you sure you want to hide this comment? It will become hidden in your post, but will still be visible via the comment's permalink.
Hide child comments as well
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.