Everyone's A Critic
The Why And How of Code Review
Malcolm Young
Senior Software Engineer
What is Code Review?
Pull Requests
 |
Thanks Alex |
one who makes no mistakes makes nothingGiacomo Casanova
Prevent bugs
Improve maintainability
Tools for the job
 |
 |
 |
|
What Are We Looking For?
- Will It Work?
- Security
- Performance
- Accessibility
- Maintainability
Never Trust User Input
Sanitise Everything
Performance
Unnecessary sessions?
Caching
Inefficient loops
Accessibility
Keyboard users
Hiding text appropriately?
Hover and focus
Maintainability
Reinventing the wheel
Appropriate API usage
Unit test coverage
Comments
Code smells
Larry Garfield at DrupalCon London 2011 https://www.youtube.com/watch?v=Z78ZP_UtaKE
- And
- Or
- If
- Testability
- Documentation
- Tight Coupling
- Impurity
Custom v contrib
Don't make it personal
Set up your IDE:
https://www.drupal.org/docs/develop/development-tools
Pre-commit hooks:
https://github.com/geraldvillorente/drupal-pre-commit
Static analysis with Jenkins:
http://reload.github.io/jenkins-drupal-template/
Small is Beautiful
10 lines of code = 10 issues.
— Michal Konturek (@MichalKonturek) June 22, 2015
500 lines of code = "looks fine".
Code reviews.
Image credits
Further Reading
Everyone's A Critic The Why And How of Code Review Malcolm Young Senior Software Engineer @malcomio https://malcomio.github.io/presentations/code-review Large teams, small teams, even solo workers
I'm not going to give you all the answers in this talk, but hopefully I'll help you ask the right questions
Invite questions as we go, time at end
First question is a basic one...