Why we are doing code reviews?
- Ensure code meets standards
- Find bugs
- Share knowledge (one of the most important)
- Check code is understandable
- Ensure the code does what it’s supposed to
When the review is complete?
- When does everyone agree?
- When a gatekeeper agrees?
- When all comments are addressed?
Pair programming
If your pairing you don’t need code reviews, for that reason in extreme programming pairing it’s a good thing to perform.
What you should be looking for in a code review?
- Fit with the overall architecture.
- SOLID Principles, DDD, Design Patterns, etc.
- New Code should follow the team’s current practices.
- Cide is in the right place?
- Code reuse
- Over-engineering?
- It’s readable code and well-tested?
- Testing the right things.
- Exceptions error messages.
- Subtle bugs
- Security
- Business requirements
- Performance issue?
- It’s well documented and/or documented files related have been updated?
- Spelling, punctuation & grammar on user messages.
- Code reviews should not focus on tools that can be automated like a static analyzer (eg: eslint), as a syntax check.
Understand the constraints
If you are for example in a secure area, your system is isolated, probably your code should not behave in the same security constraint that could have another system with less security. Another example is if you are working on an application that you don’t care about the latency, you will not worry about the milliseconds, instead of seconds maybe.
Why: Knowledge Sharing
- The focus is on how easy it is to understand the code
- The purpose is not to reject the code
- The focus’s the author gets feedback in order to become the code more understandable.
When: At the end
- It’s not the right time.
- It’s late for design.
Should have specific checks to define when it’s done.
- A list of checks to go walk through.
- Clear when it’s done.
Automate Everything You Can
- Human code reviews are spending
- Format code
- Build
- Performance
- Unit
- Integration
- CI deployment
In order for reviewers don’t worry about that stuff.
Submitting for review
- Reviews should be small (in order to be the focus and quickly)
- Annotate your code ( It’s useful to annotate your code with the things you were thinking when you made those decisions)
Reviewing
Should be clear Who is reviewing?
It’s important to respond in a timely fashion. It’s important to respond quickly to avoid context switch for that person, in order to make it faster.
The goal is not to stop the process to go the PR to the marge master, go to staging, go to production, and go to the user.
Checklist of WHAT to look for, in your PR for example.
It’s useful to make a checklist in order to be clear about what they need to look for your reviewer.
Comments in the code review
- Bear in mind, why, when, and what (Be specific, so “I don’t like that” doesn’t make sense, but I don’t like the way because it’s difficult to understand, because It’s gonna be not a great performance or something specific.)
- Be nice and constructive and specific: “It’s a good idea to reward if something should look great! And don’t only because it’s something wrong”.
- Accept or Reject
