Matt Gifford aka coldfumonkeh | Consultant Developer
View Github Profile


CFCamp 2015 Day Two

Oct 23, 2015

CFCamp 2015

Monkeh Works are at CFCamp in Munich and will be posting notes of sessions attended throughout the day.

Thanks to Bluegras.de for organising this great event and for all of the sponsors for, well, sponsoring it.

Day 2

This is the brain dump from day two of the CFCamp conference and notes taken throughout the sessions attended.

Make of them what you will. May contain spelling mistakes.


How to stop wasting your time and start performing useful code reviews - Maria Khalusova

Code reviews help with:

  • growing as a developer
  • on-boarding new team members
  • keeping track of changes
  • building better software
  • increasing bus factor
  • sharing knowledge
  • finding bugs
  • code maintenance
  • team collaboration

Don’t use code reviews as performance reviews

Starting of

Key Aspects:

The Team
  • Communicate clearly
  • Listen to concerns
  • Cultivate code review culture
    • Share best practices
    • Create a guideline or checklist document
    • Work together to achieve this so everyone feels involved
Change Impact
  • Who will be doing the code reviews?
    • Depends on the size of your organisation
    • Pick early adopters of the process as you gradually roll out
    • Who will be the reviewers? Only seniors? Everyone?
  • What?
    • What project/s will be reviewed? All of them? Only core?
    • What areas?
    • Best to start with the new commits. Don’t work back through historical archive
  • When
    • Pick suitable time to start
    • Not right before a big release (or public holiday)
Process
  1. Work on feature / fix
  2. Commit the Change
  3. Review the change
  4. Done?
    • If no, repeat steps 1-3
  • Decide on the process. It doesn’t have to be fixed to above
  • Keep workflow simple. Don’t over-complicate the rules. They’re easier to break if too complicated
  • Iterations are good
  • Best practices suggest average of 2 reviewers
  • Review often
    • Keeps the practice moving and iterating
Tool
  • You need a code review tool
  • Find the right tool for you:
    • Fits your environment
    • Supports chosen workflow
    • Meets your particular needs
    • Keeps you in the loop without spamming

Git repositories should NEVER have problems finding a tool that works.

So..
  • Have an open dialog with the team
  • Have a good plan
  • Find a tool that works

Making code reviews useful

Automate whatever can be automated

  • Tests
  • Continuous Integration
  • Static code analysis
  • Spellchecker

** Code review is NOT a place for coding style wars**

Don’t waste time discussing or arguing about spaces / tabs, styles etc.

(nb: use editor config)

As code author
  • Review your own code
    • Double-check before you commit - can drastically reduce errors
  • Commit small changes
    • Makes reviews easier and quicker
    • Helps to avoid merge conflicts
  • Document your code and write meaningful commit messages
As code reviewer
  • Don’t delay the review
    • You don’t need to jump on it straight away but don’t hold back work either
  • Don’t spend too much time
    • 60-90 minutes is maximum you SHOULD be spending
    • If you cant find a bug (or potential within that time) move on - don’t waste time
  • Apply your expertise
    • You may have specific skills - use them when reviewing and help others improve

Know what to look for

General and Business Logic
  • Correctness (does it do what it’s supposed to)
  • Coding errors (variable names etc)
  • Business logic and rules
  • Check user-facing messages
    • Are they correct and clear?
Architecture and Design
  • Is the code in the right place?
    • Can it be moved?
    • Over-engineered?
    • Too complex?
    • Will it impact future project plans regarding structure and scope?
  • Complexity
  • Reusability
  • Data Structures
Readability and Maintainability
Always code as if the person who ends up maintaining your code is a violent psychopath who knows where you live - John F. Woods (1991), Martin Golding (1994)
  • Naming
  • Readability
  • Test coverage
    • 100% is unrealistic for ANY team
    • Have your key critical areas
  • Documentation
Performance

Automate these where possible:

  • Performance Requirements
  • Performance Tests
  • Unnecessary network calls
  • Potential memory leaks
Security
  • Review potential problems
  • Third-party libraries
  • Authentication
  • Data encryption
  • Proper management of password, encryption keys and other secrets?

Useful resources:

  • Common weakness enumeration: cwe.mitre.org
  • owasp.org code review guide book: https://www.owasp.org/index.php/OWASP_Code_review_V2_Table_of_Contents

Learn to give feedback

  • Make it constructive
  • Don’t be rude
    • Strong language will not highlight or help the issue or level of severity
  • Don’t dictate - ask questions and engage in a discussion
  • It’s OK to disagree and argue
    • Be constructive with your arguments
    • Don’t argue just because it’s not exactly how you would do it
  • Do not teach
    • Learn though code reviews by asking questions and getting answers
    • Don’t dictate or impose solutions upon others
  • Be sensitive to cultural differences
  • Contain your immediate reaction
  • Consider suggestions
  • Ask followup questions if you don’t understand
    • Or clearly explain your reasons

** Praise Good Work**

We need more positive feedback.



Latest Blog Posts

Oct 21, 2019
Property Value Shorthand proposition for CFML
Read More
Oct 17, 2019
Lucee 5 - Breaking out of script into tags and back again
Read More