Code Submission Quality Criteria¶
Coding style still needs more documentation. Working on that here: Coding Style
Also, check the existing code. If yours is wildly different, it's wrong.
- commits need to be logically separated. Don't fix unrelated things in one commit.
- don't add unnecessary commits, if commit 2 fixes commit 1 merge them together (squash)
- commits need to have proper messages, explaining anything that is non-trivial
- commits should not at the same time change, rename and/or move code
- documentation updates should be in their own commit (not mixed with code commits)
- commit messages need to be properly formatted:
- meaningful and short (50 chars max) subject line followed by an empty line
- description, wrapped at ~72 characters
- ticket it fixes. E.g. "Fixes Bug #123."
- compiler warning it addresses
- Coverity Scan issue it addresses
- static analyzer error it fixes (cppcheck/scan-build/etc)
- Naming convention: prefix message with sub-system ("rule parsing: fixing foobar").
A github pull request is actually just a pointer to a branch in your tree. Github provides a review interface that we use.
- a branch can only be used in one single PR
- a branch should not be updated after the pull request
- a pull request always needs a good description (link to issue tracker if related to a ticket).
- incremental pull requests need to link to the prior iteration
- incremental pull requests need to describe changes since the last PR
- link to the ticket(s) that are addressed to it
- links to the prscript builds*
- pull requests are automatically tested using travis-ci (https://travis-ci.org/inliniac/suricata/pull_requests). Failing builds won't be consider and should be closed immediately.
- pull requests that change, or add a feature should include a documentation update commit
(*) access to prscript is limited to trusted devs. For the rest of you, ask Victor, Eric, Andreas or Jason to run it for you.
Tests and QA¶
As much as possible, new functionality should be easy to QA.
- add unittests where possible
- provide pcaps
- provide example rules if the code added new keywords or new options to existing keywords