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
- Work on feature / fix
- Commit the Change
- Review the change
- 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.