Best Practices

Code Review

All patches that go into a project repo need to be code reviewed by someone other than the original author. Code review is a great way to both learn from others as well as improve code quality. Contribution to code review is highly recommended regardless of activity as a committer.

Below provides a simple checklist of common items that code reviewers should look out for (Patch submitters can use this to self-review as well to ensure that they are not hitting any of these):

General

  • Does the Git commit message sufficiently describes the change? (Refer to: https://chris.beams.io/posts/git-commit/)
  • Does the commit message have an ‘Issue: <someissue>’ in the footer and not in the subject line or body?
  • Are there any typos?
  • Are all code review comments addressed?
  • Is the code rebased onto the latest HEAD of the branch?
  • Does the code pull in any dependencies that might have license conflicts with this project’s license?

Code

  • Are imports alphabetical and sectioned off by stdlib, 3rdparty, and local?
  • Are functions / methods organized alphabetically? (or categorized alphabetically)
  • Does the change need unit tests? (Yes, it probably does!)
  • Does the change need documentation?
  • Does every function added have function docs? (javadoc, pydoc, whatever the language code docs is)
  • Does it pass linting?
  • Does the code cause backwards compatibility breakage? (If so it needs documentation)

Note

Refer to Google’s blog (google-blog-code-health) on effective code review.

Generic Linting (Coala)

Coala is a great tool for linting all languages. The easiest way to run Coala is with python-tox and requires Python 3 installed on the system:

tox -ecoala

Running Coala without Tox can come in handy for executing Coala in interactive mode. In this case, install Coala in a Python viritualenv. Use virtualenvwrapper as it makes it simple to manage local virtual environments.

Requirements

Install Coala

Note

Some distros have a package called coala available but do not confuse this package with python-coala which is an entirely different piece of software.

Using virtualenv (assuming virtualenvwrapper is available), install Coala:

mkvirtualenv --python=/usr/bin/python3 coala
pip install coala coala-bears
coala --help

For future usage of an existing virtualenv, activate as follows:

# Re-activate Coala virtualenv
workon coala
# Run the coala command
coala --help

Set up Coala for a Project

Use python-tox to manage a Coala setup for any projects that require linting.

Requirements

Configure the project with a tox.ini and a .coafile file. Below are examples of .coafile and tox.ini as defined by lftools. Inside the tox.ini file the interesting bits are under [testenv:coala].

.coafile

[all]
ignore = .tox/**,
    .git/**,
    .gitignore,
    .gitreview,
    .gitmodules,
    node_modules/**

[all.Git]
bears = GitCommitBear
ignore_length_regex = Signed-off-by,
    Also-by,
    Co-authored-by,
    http://,
    https://

[all.Documentation]
bears = WriteGoodLintBear
files = docs/**/*.rst

[all.MarkDown]
bears = MarkdownBear,SpaceConsistencyBear,WriteGoodLintBear
files = **.md, **.markdown
use_spaces = true

[all.Python]
bears = BanditBear,
    PEP8Bear,
    PyCommentedCodeBear,
    PyDocStyleBear,
    PyFlakesBear,
    PyImportSortBear
files = *.py

tox.ini

[tox]
minversion = 1.6
envlist =
    check-best-practices,
    check-hooks,
    coala,
    docs,
    docs-linkcheck
skipsdist=true

[testenv]
install_command=python -m pip install --no-cache-dir {opts} {packages}

[testenv:check-best-practices]
commands = python {toxinidir}/check-best-practices.py

[testenv:check-hooks]
deps = pre-commit
commands =
    pre-commit install
    pre-commit run --all-files

[testenv:coala]
basepython = python3
deps =
    coala
    coala-bears
    detox~=0.18
    nodeenv
    pygments~=2.3.1
    # request-2.22.0 does not work with python-3.4.9
    requests~=2.21.0
commands =
    nodeenv -p
    npm install --global remark-cli remark-lint write-good
    python3 -m nltk.downloader punkt maxent_treebank_pos_tagger averaged_perceptron_tagger
    coala --non-interactive

[testenv:docs]
deps = -rrequirements.txt
commands =
    sphinx-build -j auto -W -b html -n -W -d {envtmpdir}/doctrees ./docs/ {toxinidir}/docs/_build/html

[testenv:docs-linkcheck]
deps = -rrequirements.txt
commands = sphinx-build -j auto -W -b linkcheck -d {envtmpdir}/doctrees ./docs/ {toxinidir}/docs/_build/linkcheck

GitHub Workflow

When working directly on Github (as opposed to Gerrit systems mirrored to Github), you’ll need to create a fork and use branches/ pull requests to get changes merged to the main repo. Here are some instructions on creating and maintaining your fork.

Forking and working

  1. Fork your $PROJECT/$REPO to your personal Github account

    • NOTE if you are forking the ci-management repository you should consider
      changing the local fork name after you have forked it to be $PROJECT-ci-management this has 2 benefits:
      1. Let you know which upstream project the ci-management repo it’s for
      2. Allow you to fork the next ci-management repository that you might need to work on
  2. Clone your repo

    git clone git@github.com:$MYACCOUNT/$REPO
    
  3. Setup an upstream remote

    git remote add upstream git@github.com:$PROJECT/$REPO
    
  4. Create local branch and do your work (same as with gerrit)

    git checkout -b $feature
    
  5. Push your local branch to your fork, preferably as a branch on your fork

    git push origin $feature
    
  6. Raise PR against the upstream (note: when pushing a branch from your local to your fork the CLI gives you a URL for raising the PR)

Care and feeding of your fork

Your fork will fall out of sync with the upstream repo so be sure to tend
to your fork before working on it.
  1. Fetch upstream changes from the remote you’ve already added:

    git fetch upstream
    
  2. Switch to primary branch and refresh your master branch

    git checkout master && git pull --rebase upstream master
    
  3. Update Github with your synced fork:

    git push origin