- Title
- Important
- Urgent
- Definition of deliverables
- Assignee
- Estimate of effort required
- Deadline
- If a ticket is in a blocked state, then the reason why it is blocked should be made explicit
- One liner describing what changed (not period terminated)
- A few lines describing in more details why things changed
- GPG signed commit
- Separate subject from body with a blank line
- Limit the subject line to 50 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line
- Wrap the body at 72 characters
- Use the body to explain what and why vs. how
When joining a new project without tests, here is the value you need to provide through the addition of tests:
- the application works and doesn't crash
- the application works and supports a few input cases
- the application works and supports a variety of input cases
- the application works and is robust to most input cases
- Write a test that tests the common case usage of your function
- Write tests that cover edge cases of your function
- Write tests to cover all statements, branches, paths
- Code changes are stored in git
- Setup continuous integration
- Have a testing framework
- Use dependency management
- Define a code standard
- Prefer function/method typing over dynamic types
- On every push to git
- Code quality check
- Code style check
- Unit/functional/integration/system tests
- Code coverage should be recorded during tests and a report made available
- A project repository must have a README.md explaining how to run the project on your own computer
- A project repository must have a RELEASING.md explaining how to release the code
- Responsibilities are made explicit in terms of roles
- Critical roles, such as project lead, must have a backup/shadow individual
- Setup telemetry, alerts, profiling, logging
- Verify that the build/tests pass
- Read the issue title and description
- New code
- Understand the feature and associated requirements that are supposed to be implemented
- Verify code implements the desired feature and that the requirements are completed
- New/Changed code
- Check code contains tests
- Is all the new code covered by those tests?
- Verify the location of new/moved files
- Are the files in the right directory?
- Are they appropriately named?
- Verify classes, methods, functions, parameters naming
- Are they significant of their purpose?
- Are they clear enough?
- Are they respecting the naming convention?
- Does the code respect SOLID?
- Consider that when functions/methods signature change, code may now be backward incompatible.
- Discuss whether this is necessary
- Backward incompatible changes should be documented
- In a weak typed or type hinted language, are parameters and return of functions/methods typed?
- Are there TODOs that should be completed within this review?
- Check code for code style issues
- Check code contains tests
- Bug fix
- Verify that the fix is applied at the right location and will not "fix the symptoms, not the cause"
When reviewing
- Provide specific and actionable feedback
- Clearly mark nitpicks and optional comments
- Assume competence
- Provide rationale or context
- Consider how comments may be interpreted
- Don't criticize the person, criticize the code
- Don't use harsh language
- https://en.wikipedia.org/wiki/SOLID
- https://medium.com/palantir/code-review-best-practices-19e02780015f
- https://phauer.com/2018/code-review-guidelines/
- https://smartbear.com/learn/code-review/guide-to-code-review-process/
- https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
- https://nyu-cds.github.io/effective-code-reviews/03-checklist/
- https://google.github.io/eng-practices/
- https://testing.googleblog.com/2019/11/code-health-respectful-reviews-useful.html
- https://engineering.18f.gov/code-review/
- https://conventionalcomments.org/
- https://docs.gitlab.com/ee/development/code_review.html
- https://gitmoji.dev/