Revision as of 10:28, 23 October 2014 by Martin.grebac.oracle.com
Code Review Goals
- Spot and fix defects early in the process
- Better shared understanding of the codebase - learn from each other
- Identify common errors, reduce amount of work
- Build confidence in the overall quality of the solution/change
- Work and discuss together
- Assure the resulting product (code) adheres to best standards
Code Review Checklist
- Is the code safe?
- Will it run/fail under security manager?
- Adding new calls for reflection, class loaders, doPrivileged ... ?
- Does the change introduce any new findbugs warnings?
- Is the change sufficiently covered by tests?
- Was enough testing done for the change?
- Does the change break backwards compatibility in any way?
- Does the code work on all supported JDKs? Databases? Servers?
- Does the code break under concurrency?
- Is it thread-safe? Should be?
- Any obvious performance anti-patterns?
- Do we understand performance implications of the change on performance - cpu&memory wise?
- Does it need new performance tests?
- Is there enough logging in the code?
- E.g. - in case of tooling option changes - are we updating maven/ant plugins to include the options, too?
- Is the change consistent across the stacks? GF/WLS/..., ORADB/MSSQL/MYSQL/...
- Is the change made in moxy code consistent with jpa and vice versa?
- Any new feature should be documented, what is not documented does not exist and is not done
- Any new code should have good enough javadoc, API must have full comprehensive javadoc
- JDK 8 is more restrictive in javadoc - write javadoc jdk8 compatible
- Does the code have proper licence/copyright headers?
- Are copyright years updated?
- Is the code contributed properly - did the contributor really author 100% of the code? sign-off, ...
- Make sure all messages and logging above or equal to level INFO is coming from resource bundles
- Follow proper I18N rules (do not combine bundles, ...)
Code style, Formatting
- Try to keep line-endings intact on unchanged lines/files
- Eclipse: $REPO/project-admin/EclipseLink-Eclipse-Format.xml