All code needs to be reviewed by a second engineer before being merged into master.Â
We don’t have hard and fast rules on how we do code reviews. For more complex tickets it might make sense to do a walk-through, where the author explains his solution to the reviewer. For less complex tickets it might be easier for the reviewer to do the review by himself. It’s up to you to decide what feels appropriate for the given PR.
Some points to make Code Reviews run smooth:
Keep issues in separate branches for easier separation of changes in the review
Do reviews between issues ie. do reviews before starting on the next issue
Always find the reviewer immediately when finishing up the issue and schedule a time slot if the review needs explaining
Add a comment to the issue with implementation overview, what should work and what old functionality is in the risk or breaking ie. what is affected by the new changes, a link to the ticket is enough if everything the reviewer needs to know can be found in there.
Merge master to feature branch and run tests before review
The concept behind rubber ducking or rubber duck debugging is simple yet effective: by explaining your code or a problem you’re trying to solve to someone else or even an inanimate object (like a rubber duck), you gain a deeper understanding of the problem and might even hit upon the solution in the process of explaining.
While this is mostly used for debugging, we encourage you to also use this technique before starting to implement a complex task or when you are stuck in between.
Please try solving the issue on your own first (we are happy to provide you with a rubber duck), before consulting someone else but don’t be afraid to interrupt another engineer if you need help. Find someone and explain what you’re trying to solve and how you think you want to approach it and you might even get a second opinion or a pair programming session out of it.