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
Code Review¶
Please read 11 Best Practices for Peer Code Review.
See also Code reviews: the lab meeting for code and the PyCogent coding guidelines.
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);`.