Coding guidelines and code review checklist

This document is for anyone who want to contribute code to the khmer project, and describes our coding standards and code review checklist.

C++ standards

Any feature in C++11 is fine to use. Specifically we support features found in GCC 4.8.2. See https://github.com/dib-lab/khmer/issues/598 for an in-depth discussion.

Coding standards

All plain-text files should have line widths of 80 characters or less unless that is not supported for the particular file format.

For C++, we use Todd Hoff's coding standard, and astyle -A10 / "One True Brace Style" indentation and bracing. Note: @CTB needs Emacs settings that work for this.

Vim users may want to set the ARTISTIC_STYLE_OPTIONS shell variable to "-A10 --max-code-length=80" and run `:%!astyle` to reformat. The four space indentation can be set with:

set expandtab
set shiftwidth=4
set softtabstop=4

For Python, PEP 8 is our standard. The `pep8` and `autopep8` Makefile targets are helpful.

Code, scripts, and documentation must have their spelling checked.

Python-based codespell can be applied to multiple files easily. codespell can be installed via the following:

mkdir ~/bin
git clone git@github.com:lucasdemarchi/codespell.git
cd codespell
make prefix=${HOME} install
export PATH=$PATH:~/bin/

Note, if you want codespell to always be available you will need to add the export line to your ${HOME}.bashrc or equivalent.

To run codespell over only what has been changed on the branch my-branch:

git diff master..my-branch > diff_file
codespell diff_file

To run codespell over a single file:

codespell path/to/file

To make codespell fix the issues it finds automatically:

codespell -w path/to/file

Please note that as codespell works off of a listing of possible misspellings it may not catch all errors. If you find a spelling error that is not caught by codespell feel free to open a pull request at the project page to add it to the dictionary.

Vim users can run:

:setlocal spell spelllang=en_us

Use ]s and [s to navigate between misspellings and z= to suggest a correctly spelled word. zg will add a word as a good word.

GNU aspell can also be used to check the spelling in a single file:

aspell check --mode ccpp $filename

Checklist

Each pull request should be automatically populated with the following checklist:

- [ ] Is it mergeable?
- [ ] `make test` Did it pass the tests?
- [ ] `make clean diff-cover` If it introduces new functionality in
  `scripts/` is it tested?
- [ ] `make format diff_pylint_report cppcheck doc pydocstyle` Is it well
  formatted?
- [ ] Did it change the command-line interface? Only additions are allowed
  without a major version increment. Changing file formats also requires a
  major version number increment.
- [ ] Is it documented in the `ChangeLog`?
  http://en.wikipedia.org/wiki/Changelog#Format
- [ ] Was a spellchecker run on the source code and documentation after
  changes were made?
- [ ] Do the changes respect streaming IO? (Are they
  tested for streaming IO?)
- [ ] Is the Copyright year up to date?

Note that after you submit the pull request you can check and uncheck the individual boxes on the formatted comment; no need to put x or y in the middle.

CPython Checklist

Here's a checklist for new CPython types with future-proofing for Python 3:

- [ ] the CPython object name is of the form `khmer_${OBJECTNAME}_Object`
- [ ] Named struct with `PyObject_HEAD` macro
- [ ] `static PyTypeObject khmer_${OBJECTNAME}_Type` with the following
  entries
   - [ ] `PyVarObject_HEAD_INIT(NULL, 0)` as the object init (this includes
     the `ob_size` field).
   - [ ] all fields should have their name in a comment for readability
   - [ ] The `tp_name` filed is a dotted name with both the module name and
     the name of the type within the module. Example: `khmer.ReadAligner`
   - [ ] Deallocator defined and cast to `(destructor)` in tp_dealloc
     - [ ] The object's deallocator must be
       `Py_TYPE(obj)->tp_free((PyObject*)obj);`
   - [ ] Do _not_ define a `tp_getattr`
   - [ ] BONUS: write methods to present the state of the object via
     `tp_str` & `tp_repr`
   - [ ] _Do_ pass in the array of methods in `tp_methods`
   - [ ] _Do_ define a new method in `tp_new`
- [ ] PyMethodDef arrays contain doc strings
   - [ ] Methods are cast to `PyCFunctions`s
- [ ] Type methods use their type Object in the method signature.
- [ ] Type creation method decrements the reference to self
  (`Py_DECREF(self);`) before each error-path exit (`return NULL;`)
- [ ] No factory methods. Example: `khmer_new_readaligner`
- [ ] Type object is passed to `PyType_Ready` and its return code is checked
  in `MOD_INIT()`
- [ ] The reference count for the type object is incremented before adding
  it to the module: `Py_INCREF(&khmer_${OBJECTNAME}_Type);`.
comments powered by Disqus