Style Guide - lanl/Draco GitHub Wiki

This guilde should also be used as a Code Review Checklist.

Design by Contract

  • Functions begin with Require assertions that test acceptability of each input parameter
  • Functions begin with Require assertions that test preconditions for any other state involved in the function
  • Functions end with Ensure assertions that test acceptability of each output parameter or return value
  • Functions end with Ensure assertions that test postconditions for any other state involved in the function

Tests

  • Whenever possible, the lowest level of testing for a given change is at the same level as the change itself. For example, the lowest-level test for a change in c4 is in c4/test, not quadrature/test.

Comments

  • Comments explain current state of the code, not history of changes
  • Comments begin at the same level of indentation as a line of code would, on a line by themselves

Comments about functions

  • Function declarations begin with a Doxygen brief description, focusing on a consumer's view of the function (e.g., its purpose and expected usage)
//! Function foo does X.
  • Function definitions begin with a Doxygen comment that describes a tester's view of the function (e.g., how outputs change based on inputs or other state) ** Emacs (M-x draco-insert-function-doc)
//-----------------------------------------------------------------------------------------------//
/*! 
 * \brief 
 * 
 * \param[in] name description
 * \return description
 */
  • Comments above function definitions also provide a maintainer's view of the function (e.g., rationale for implementation decisions)
  • The Draco documentation system page for each changed file is complete

Comments about parameters

  • Comments above function definitions include a Doxygen \param command for each parameter
  • Constraints on input parameters or expectations for output parameters are captured as Design by Contract assertions, rather than comments about the parameter, whenever possible

Comments about classes

  • Classes begin with a Doxygen comment covering both a consumer's view (e.g., its purpose and expected usage) ** Emacs: M-x draco-insert-class-doc
  • ... and a maintainer's view (e.g., rationale for implementation decisions)

Formatting

  • Lines wrap at 80 characters (because otherwise-productive tools like side-by-side diff and merge tools are difficult to use when lines are long)
  • Indentation matches the Draco emacs mode and is accomplished with spaces, not tabs
  • Blank lines are present when they increase readability but don't waste vertical space
  • Extra whitespace within a line, for alignment, is consistent through the block where it appears

Commit messages

  • Commit messages explain history---what previous decision was altered to warrant the change being made
  • Commit messages include a reference to an issue, when one exists
  • Git commit messages should have the format:
One line summary (less than 80 chars).
(blank line)
Detailed discussion or bullet list.

Automatic formatting tests

  • git-clang-format is used to enforce some basic formatting rules. Each pull request must satisfy this check. Only lines of code modifed by the PR are checked.
  • If git-clang-format is available on your machine (e.g. ccs-net machines), you can run regression\check_style.sh -d to see if your local changes meet the formatting requirements.

Additional checks for Pull Requests

  • Each pull request must increase (or at least not decrease) the code coverage measurement.

  • Each pull request must pass a valgrind build, a Toss2 build and a CrayPE build. Whoever is reviewing the PR must complete these checks are report the results. For example:

    • Luna, Snow, Trinitite: % /usr/projects/jayenne/regress/draco/regression/checkpr.sh -p draco -f pr42
    • ccscs[27]: % /scratch/regress/draco/regression/checkpr.sh -p draco -f pr42

where 42 should be replaced by the actual pull request identifier. Results will appear on CDash.