Practical
Code Reviews
Ilko Kacharov
Experience
Lead Dev @Proxiad / @Mailjet
12+ years in PHP
Zend Framework 2 Certified Architect
Engineer in communications
4+ years in Code reviews
Home
Shiny
Frontend
Messy
Backend
My Project is my Home
~8 hours a day
Clean and tidy
Secure and private
My Project is my Home
When you start a project you define the rules of the house.
Everyone who joins the project
should follow the rules!
My Project is my Home
Don't end up here
The housekeeper
Artwork by
Back in the days
2015+
Maintainability and Simplicity
Code management
2010
Code quality / style / standards, best practices
2005
Browser wars and internet revolution - Web 2.0
Why
do we need the process
- Reading code gets you in the mood for coding
- Keeps you up to date with the codebase
- Reduces back and forth with QA team
- Provides early feedback on the code
- Coding becomes social activity
- Starts discussions and generates ideas
What
- Code duplication
- Functionality duplication
- Introducing or reducing technical debt
- Bugs and inconsistencies
- Dependencies to libraries and repos
- Security and sensitive data protection
- Performance bottlenecks
- Maintainability
to look for in the code
Artwork by
When
You know the code
- Suggest architecture improvements
- Add code snippets (grabs attention, legitimacy, facts)
to comment the code
You don't know the code
- Ask for details or explanation (you might learn something)
- Too obvious things to ignore
Gut feelings & raised eyebrows
- Check your sources first (links & references to source code)
-
Uncontroversial approach
How
to be formally polite
Proposals
"We can use a tool instead of doing it manual."
"We better do filtering before we insert in the database."
"You can check if this code is available already."
Artwork by
How
to be formally polite
Questions
"We've changed this recently. Have you seen it?"
"Have you checked the code in the other module?"
"Is this code gonna be able to do the math if we pass 0?"
How
to be formally polite
Appeals
"Please verify that input, so we include all edge cases."
"Please use Оctocat memes when doing web presentations."
"Please add exclamation marks when code must be changed!"
Artwork by
How
to be formally polite
Positivity
"Good catch!"
"Clever approach!"
"Never knew code can do that!"
Artwork by
How
to be formally polite
Opinions are last
"I think this code already exists."
"I think we can do backflips instead."
The one who gives only opinion takes no responsibility.
To improve is to change
To perfect is to change often
Winston Churchill
«
»
Artwork by
Iterations and passes
- Daily code review routine
- Code review checklist for consistency
- Monitor and track security patterns
- Review individual commits on meaningful changes
- Early feedback on general issues
~ 40 lines
~ 200 lines
> 500 lines
First feedback at 30%
- Early feedback on the approach
- Focus on architecture decisions
- Skip minor details, code style
Second feedback at 90%
- Final touches
- Code style and trivial things
- Configurations and environment
Tools
-
Diff
-
History
-
Blame
-
Ignore whitespace
-
Ticket references #
-
Pre-commit hooks
-
Pull Requests
-
Labels (ask me later)
-
Code comments
-
Inline resolving of conflicts
-
Automatic code quality tools
-
Slack / Email notifications
to improve the process
Artwork by
Tools
to improve the process
Slack Notifications in #git-repo-name
Thank you!
@kachar136
https://slides.com/kachar/practical-code-reviews-bws/
Questions?
@kachar136
Thanks to GitHub.com for all the Octocats
Practical Code Reviews [BWS]
By Ilko Kacharov
