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

Made with Slides.com