Downloaded 30 times
Uploaded byTrisha Gee
Code Review Matters and Manners
The document discusses best practices for code reviews, focusing on key areas such as formatting, design, usability, and functionality. It emphasizes the importance of maintaining proper manners during reviews by providing constructive feedback and fostering a collaborative environment. The authors stress the need to automate checks where possible and to prioritize significant aspects without becoming overly critical of minor details.
More Related Content
Similar to Code Review Matters and Manners
Code Review Matters and Manners
- 1. Trisha Gee (@trisha_gee) MariaKhalusova (@mariaKhalusova) JetBrains Code Review Matters and Manners
- 2.
- 6. What To LookFor • Formatting • Naming Conventions • “Final” Keyword • Line Length • Empty Statements • Modifier Order • Javadoc comments • ….
- 7. Automate these checks Don’tSweat the Little Things
- 8. Design: Details • Isthe code in the right place? • Appropriate data structures used? • Have opportunities for code re-use been taken? • Have new dependencies been introduced? • Is the code making use of existing libraries? • Is it utilising new idioms?
- 9. Design: High Level •Does the new code fit with the overall architecture? • Does this new code follow the current design practices? • Have appropriate design patterns been used? • Does the code follow SOLID principles, Domain Driven Design or other preferred approaches? • Is the code overly complex or over-engineered?
- 10. Do it inadvance, or as you go Code Review is Too Late For Design
- 11.
- 12.
- 13. Usability • Readability • Maintainability •Extensibility
- 14. Functionality • Does itdo what it’s supposed to? • Does it meet performance requirements? • Does it meet security requirements? • Are there other regulatory requirements?
- 15. Decide on priorities,apply consistently Understand what’s important
- 17. Automate these checks Don’tSweat the Little Things
- 22. Decide on priorities,apply consistently Understand what’s important
- 25. Do it inadvance, or as you go Code Review is Too Late For Design
- 26. Don’t Sweat theLittle Things
- 27. Understand What’s Important •Functionality • Does it do what it’s supposed to? • Does it meet “non-functional” requirements? • Usability • Readability • Maintainability • Extensibility
- 28. Understand What’s Important •Is the code in the right place? • Are the data structures that have been used appropriate? • Have opportunities for code re-use been taken? • Is the code overly complex or over-engineered? • Have new dependencies been introduced? • Is the code making use of existing libraries? • Is it utilising new idioms from language updates?
- 29. Code Review isToo Late for Design • Is the code in the right place? • Are the data structures that have been used appropriate? • Have opportunities for code re-use been taken? • Is the code overly complex or over-engineered? • Have new dependencies been introduced? • Is the code making use of existing libraries? • Is it utilising new idioms from language updates?
- 30. Code Review isToo Late for Design • Does the new code fit with the overall architecture? • Does this new code follow the current design practices? • Which design patterns are used in the new code? Are these appropriate? • Does the code follow SOLID principles, Domain Driven Design etc?
- 31. Does it dowhat it’s supposed to?
- 32. Is anything wildlywrong with it?
- 33.
- 34. How to havecode review discussions without starting a war in the comments?
- 36. This happens… “Instead oftouching other people’s code, do something useful with your life” “Look at the bullshit you wrote” “The above code is shit, and it generates shit code. It looks bad and there’s no reason for it”
- 38.
- 39. It’s not onlyabout being polite… “Comments must end with a period” “Something is wrong. I’m not sure what it is. It just doesn’t feel right, you know what I mean?”
- 40. Why do youleave feedback? There’s a problem To help someone improve To start a discussion To praise good work Because of the pressure to find a problem To boost own ego
- 41. Basic code reviewmanners Appropriate timing and place Indicate when you’re done with a review Discuss changes, not people Be specific Don’t demand, ask questions Don’t use sarcasm Suggest alternatives
- 42. A few tipson wording Don’t be rude. “WTF is this?” “That’s a dumb idea…” Who, What, Where, How, And Why? “This will not work if…” vs “What happens if..?” Avoid using “obviously”, “simply”, “just” Avoid possessive adjectives “Your method returns…” vs “This method returns…” Never say never (and “always”, “nothing”, etc.)
- 43. There’s a catch:) “Look at the bullshit you wrote”
- 44. “A fool thinkshimself to be wise, but a wise man knows himself to be a fool.” William Shakespeare “Everyone you will ever meet knows something you don't.” Bill Nighy
- 45. Receiving feedback Invite teammatesto review your code Separate criticism from self Immediate reaction isn’t always the best one Ask questions Be grateful for the feedback
- 46. Trisha Gee (@trisha_gee) MariaKhalusova (@mariaKhalusova) JetBrains Questions? http://bit.ly/CR-MM/