...
2.1 Grammar and style
These are small things that are not caught by the automated style checkers.
- Does a variable need a better name?
- Should this be a keyword argument?
- In a PR, maintain the existing style of the file.
- Don’t combine code changes with lots of edits of whitespace or comments; it makes code review too difficult. It’s okay to fix an occasional comment or indenting, but if wholesale comment or whitespace changes are needed, make them a separate PR.
- Use the checkstyle plugin in Maven to verify that your PR conforms to our style
2.2 Code Style
- Follow the Google Java Style Guide outlined here: https://google.github.io/styleguide/javaguide.html
2.2.1 IntelliJ IDEA Checkstyle Warnings
- Install the Checkstyle-IDEA plugin (Available in IntelliJ's directly from Preferences -> Plugins)
- Go into Settings -> Other Settings -> Checkstyle
- Change "Checkstyle version" to 8.0.
- Apply. Otherwise the new file won't match the version and an error will be thrown.
- Add a new Checkstyle file
- Use the checkstyle.xml file included in the root directory of the project.
- If you have errors, you most likely need to make sure you're set to the right version and have applied it.
- Select the checkbox for the new style
- Apply
New, Checkstyle-based, warnings should show up in Java files, e.g.
Checkstyle: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement
2.2.2 IntelliJ IDEA Code Formatting
- Download https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml
In IDEA, go to Preferences -> Editor -> Code Style.
- Click in the Settings gear next to the Scheme selection box. Go to Import Scheme -> IntelliJ IDEA Code Style XML
- Choose the previously downloaded XML file.
- It's also possible to import the Checkstyle file directly (instead of Import Scheme -> IntelliJ IDEA Code Style XML use Import Scheme -> Checkstyle Configuration). In practice, the Google IntelliJ IDEA settings appear to be more complete.
2.2.3. Eclipse Checkstyle Warnings
- Install Checkstyle from Eclipse Marketplace
- Follow the instructions for importing a custom checkstyle at http://eclipse-cs.sourceforge.net/#!/custom-config
2.2.4 Eclipse Code Formatting
- Download https://github.com/google/styleguide/blob/gh-pages/eclipse-java-google-style.xml
- In Eclipse, go to Window/Preferences and select Java/Code Style/Formatter
- Select Import and import the downloaded file.
2.3 Coding Standards
Implementation matches what the documentation says
Logger name is effectively the result of Class.getName()
Class & member access - as restricted as it can be (subject to testing requirements)
Appropriate NullPointerException and IllegalArgumentException argument checks
- Asserts - verify they should always be true
- Look for accidental propagation of exceptions
- Look for unanticipated runtime exceptions
- Try-finally used as necessary to restore consistent state
Logging levels conform to Log4j levels
- Possible deadlocks - look for inconsistent locking order
- Race conditions - look for missing or inadequate synchronization
- Consistent synchronization - always locking the same object(s)
- Look for synchronization or documentation saying there's no synchronization
- Look for possible performance problems
- Look at boundary conditions for problems
- Configuration entries are retrieved/set via setter/getter methods
- Implementation details do NOT leak into interfaces
- Variables and arguments should be interfaces where possible
If equals is overridden then hashCode is overridden (and vice versa)
Objects are checked (instanceof) for appropriate type before casting (use generics if possible)
- Public API changes have been publicly discussed
- Use of static member variables should be used with caution especially in Map/reduce tasks due to the JVM reuse feature
2.4 Documentation
Code-Level Documentation
- Self-documenting code (variable, method, class) has a clear semantic name
- Accurate, sufficient for developers to code against
- Follows standard Javadoc conventions
- Loggers and logging levels covered if they do not follow our conventions (see below)
- System properties, configuration options, and resources covered
- Illegal arguments are properly documented as appropriate
- Package and overview Javadoc are updated as appropriate
- Javadoc comments are mandatory for all public APIs
- Generate Javadocs for release builds
Feature-level documentation - should be version controlled in github in README files.
- Accurate description of the feature
- Sample configuration and deployment options
- Sample usage scenarios
High-Level Design documentation - architecture description and diagrams should be a part of a wiki entry.
- Provide diagrams/charts where appropriate. Visuals are always welcome
- Provide purpose of the feature/module and why it exists within the project
- Describe system flows through the feature/module where appropriate
- Describe how the feature/module interfaces with the rest of the system
- Describe appropriate usages scenarios and use cases
Tutorials - system-level tutorials and use cases should also be kept as wiki entries.
- Add to the Metron reference application documentation for each additional major feature
- If appropriate, publish a tutorials blog on the Wiki to highlight usage scenarios and apply them to the real world use cases
2.5 Tests
- Unit tests exist for bug fixes and new features, or a rationale is given in JIRA for why there is no test
- Unit tests do not write any temporary files to /tmp (instead, the tests should write to the location specified by the test.build.data system property)
...