Demystifying Code Reviews

What to look for when reviewing someone else’s code?

Transcript

1. Demystifying Code Reviews Hitesh Das 02-02-2021
2. What do in common code reviews look like? - Formatting? - Style? - Naming? etc. etc. WE
3. Blocking - Cross-team code reviews. Immature Guidelines. Omissible - Most common. Window dressing activity. Value Reviews - Common for teams that know and v benefits of code reviews. Power Reviews - 100/100 - How can we reach the state of power reviews?
4. What to look for when reviewing someone else’s code? 1. Design - Architecture - S.O.L.I.D principles’ scope - Long if/else statements - Casting to a subtype - Many public methods - Implemented methods that throw UnsupportedOperationException - Design Patterns scope. - Guidelines set by the team. - Is this the right place for this piece of code? - DRY compliance - Over Engineering.
5. What to look for when reviewing someone else’s code? 2. Readability - Naming conventions - Can I understand the code while reading? - Can I understand the exceptions caught? - How the complex logic is written into smaller testable units? - Maintainability with code coverage - Do the tests at least cover confusing or complicated sections of code? - Do the tests match the requirements?
6. What to look for when reviewing someone else’s code? 3. Functionality - Requirement vs Implementation - Security - Edge cases and Probable Bugs. - Environment factors - Prod. vs Dev. functionality and dependencies handling.
7. What to look for when reviewing someone else’s code? 4. Performance - Does this piece of functionality have hard performance requirements? E.g (Is the code breaking the team SLA like “all order requests serviced in less than 10 ms” - Unnecessary network calls where a single one might suffice. Mobile / wearable apps calling the back end too much and impacting battery health. - Is there something in the code which could lead to a MEMORY LEAK? - Does the code close connections/streams?
8. What to look for when reviewing someone else’s code? 4. Performance(contd...) - Use of Reflection - Whether this is absolutely required. - Is the code using a data structure with poor performance? - Complex Logging. - Timeouts. - Parallelism/Lazy loading and Race conditions - Caching Scope
9. Anti Patterns • Nit Picking - Finding little things like spacing, indentation. • Design changes when the code works - Code review at the wrong time. • Inconsistent feedback - Lack of guidelines. Reviewers biases. • The Ghost Reviewer - Big reviews. Time consuming review • Ping pong Reviews - To and fro comments and fixes backward and forward. It has been observed that, this usually happens when goal of the review is to find errors.
10. • During development • Before Code merge • After Code Merge When should code review happen?
11. • When everyone agrees • When all comments are resolved • When the gatekeeper approves When does code review completes?
12. Summary Reviewer • Comment and prioritise the comments. Clear their priority. • Bear in mind why, when, what are your reviewing • Be constructive. • Be nice
13. Summary Author • Pair with the reviewer or the gatekeeper. • Constantly seek feedback if possible at the computer. • Remote screen sharing if at different locations or not approachable in person.
14. Summary Author • Always support complex logic with supporting statements if required. • Mentioning any edge cases that has been intentionally been ignored. • If you have any future scope but not included in this version, please specify explicitly. • Be constructive and nice. • Remember, that the reviewer is your peer and he has his own priorities to take care of. • Be respectful while suggesting counter comments. • It’s equally your responsibility to get your work reviewed.
15. Getting to the POWER Code Reviews - As an author, raising small, incremental code changes in review requests. - As an author, reducing the reviewer’s burden as much as possible. - As a reviewer, increasing speed gradually to ensure faster feedback. - Last, but not the least, practice the culture of POWER Code Reviews.
16. Thank you References: 1. What To Look for in a code review - Trisha Gee 2. The Code Review Quadrant 3. Android Code Review Checklist - Droidcon Blog Remember The Goal is to ship the code