Contributing

Contributing happens via "Pull requests" (PR) on github. Every PR needs to be reviewed before it can be merged and the Continuous Integration should be green.

The PR has to be approved (and is often merged too) by one "code owner", either by the code owner who is responsible for the subsystem the PR belongs to or by two core developers or by Araq.

See codeowners for more details.

Writing tests

Not all the tests follow this scheme, feel free to change the ones that don't. Always leave the code cleaner than you found it.

Stdlib

If you change the stdlib (anything under lib/), put a test in the file you changed. Add the tests under an when isMainModule: condition so they only get executed when the tester is building the file. Each test should be in a separate block: statement, such that each has its own scope. Use boolean conditions and doAssert for the testing by itself, don't rely on echo statements or similar.

Sample test:

when isMainModule:
  block: # newSeqWith tests
    var seq2D = newSeqWith(4, newSeq[bool](2))
    seq2D[0][0] = true
    seq2D[1][0] = true
    seq2D[0][1] = true
    doAssert seq2D == @[@[true, true], @[true, false],
                        @[false, false], @[false, false]]

Compiler

The tests for the compiler work differently, they are all located in tests/. Each test has its own file, which is different from the stdlib tests. All test files are prefixed with t. If you want to create a file for import into another test only, use the prefix m.

At the beginning of every test is the expected side of the test. Possible keys are:

  • output: The expected output, most likely via echo
  • exitcode: Exit code of the test (via exit(number))
  • errormsg: The expected error message
  • file: The file the errormsg
  • line: The line the errormsg was produced at

An example for a test:

discard """
  errormsg: "type mismatch: got (PTest)"
"""

type
  PTest = ref object

proc test(x: PTest, y: int) = nil

var buf: PTest
buf.test()

Running tests

You can run the tests with

./koch tests

which will run a good subset of tests. Some tests may fail. If you only want to see the output of failing tests, go for

./koch tests --failing all

You can also run only a single category of tests. A category is a subdirectory in the tests directory. There are a couple of special categories; for a list of these, see testament/categories.nim, at the bottom.

./koch tests c lib

To run a single test:

./koch tests c <category>/<name>

E.g. ./koch test run stdlib/thttpclient_ssl

For reproducible tests (to reproduce an environment more similar to the one run by Continuous Integration on travis/appveyor), you may want to disable your local configuration (eg in ~/.config/nim/nim.cfg) which may affect some tests; this can also be achieved by using export XDG_CONFIG_HOME=pathtoAlternateConfig before running ./koch commands.

Comparing tests

Because some tests fail in the current devel branch, not every fail after your change is necessarily caused by your changes.

The tester can compare two test runs. First, you need to create the reference test. You'll also need to the commit id, because that's what the tester needs to know in order to compare the two.

git checkout devel
DEVEL_COMMIT=$(git rev-parse HEAD)
./koch tests

Then switch over to your changes and run the tester again.

git checkout your-changes
./koch tests

Then you can ask the tester to create a testresults.html which will tell you if any new tests passed/failed.

./koch tests --print html $DEVEL_COMMIT

Deprecation

Backward compatibility is important, so if you are renaming a proc or a type, you can use

{.deprecated: [oldName: new_name].}

Or you can simply use

proc oldProc() {.deprecated.}

to mark a symbol as deprecated. Works for procs/types/vars/consts, etc. Note that currently the deprecated statement does not work well with overloading so for routines the latter variant is better.

Deprecated pragma in the manual.

Documentation

When contributing new procedures, be sure to add documentation, especially if the procedure is exported from the module. Documentation begins on the line following the proc definition, and is prefixed by ## on each line.

Code examples are also encouraged. The RestructuredText Nim uses has a special syntax for including examples.

proc someproc*(): string =
  ## Return "something"
  ##
  ## .. code-block:: nim
  ##
  ##  echo someproc() # "something"
  result = "something" # single-hash comments do not produce documentation

The .. code-block:: nim followed by a newline and an indentation instructs the nim doc and nim doc2 commands to produce syntax-highlighted example code with the documentation.

When forward declaration is used, the documentation should be included with the first appearance of the proc.

proc hello*(): string
  ## Put documentation here
proc nothing() = discard
proc hello*(): string =
  ## Ignore this
  echo "hello"

The preferred documentation style is to begin with a capital letter and use the imperative (command) form. That is, between:

proc hello*(): string =
  # Return "hello"
  result = "hello"

or

proc hello*(): string =
  # says hello
  result = "hello"

the first is preferred.

Best practices

Note: these are general guidelines, not hard rules; there are always exceptions. Code reviews can just point to a specific section here to save time and propagate best practices.

Take advantage of no implicit bool conversion

doAssert isValid() == true
doAssert isValid() # preferred

Immediately invoked lambdas (https://en.wikipedia.org/wiki/Immediately-invoked_function_expression)

let a = (proc (): auto = getFoo())()
let a = block:  # preferred
  getFoo()

Design with method call syntax (UFCS in other languages) chaining in mind

proc foo(cond: bool, lines: seq[string]) # bad
proc foo(lines: seq[string], cond: bool) # preferred
# can be called as: `getLines().foo(false)`

Use exceptions (including assert / doAssert) instead of quit rationale: https://forum.nim-lang.org/t/4089

quit() # bad in almost all cases
doAssert() # preferred

Use doAssert (or require, etc), not assert in all tests.

runnableExamples: assert foo() # bad
runnableExamples: doAssert foo() # preferred

Delegate printing to caller: return string instead of calling echo rationale: it's more flexible (eg allows caller to call custom printing, including prepending location info, writing to log files, etc).

proc foo() = echo "bar" # bad
proc foo(): string = "bar" # preferred (usually)

[Ongoing debate] Consider using Option instead of return bool + var argument, unless stack allocation is needed (eg for efficiency).

proc foo(a: var Bar): bool
proc foo(): Option[Bar]

Tests (including in testament) should always prefer assertions over echo, except when that's not possible. It's more precise, easier for readers and maintaners to where expected values refer to. See for example https://github.com/nim-lang/Nim/pull/9335 and https://forum.nim-lang.org/t/4089

echo foo() # adds a line in testament `discard` block.
doAssert foo() == [1, 2] # preferred, except when not possible to do so.

The Git stuff

General commit rules

  1. Bugfixes that should be backported to the latest stable release should contain the string [backport] in the commit message! There will be an outmated process relying on these. However, bugfixes also have the inherent risk of causing regressions which are worse for a "stable, bugfixes-only" branch, so in doubt, leave out the [backport]. Standard library bugfixes are less critical than compiler bugfixes.
  2. All changes introduced by the commit (diff lines) must be related to the subject of the commit.

    If you change some other unrelated to the subject parts of the file, because your editor reformatted automatically the code or whatever different reason, this should be excluded from the commit.

    Tip: Never commit everything as is using git commit -a, but review carefully your changes with git add -p.

  3. Changes should not introduce any trailing whitespace.

    Always check your changes for whitespace errors using git diff --check or add following pre-commit hook:

    #!/bin/sh
    git diff --check --cached || exit $?

4. Describe your commit and use your common sense.

Example Commit messages: Fixes #123; refs #124

indicates that issue #123 is completely fixed (github may automatically close it when the PR is committed), wheres issue #124 is referenced (eg: partially fixed) and won't close the issue when committed.

  1. Commits should be always be rebased against devel (so a fast forward merge can happen)

    eg: use git pull --rebase origin devel. This is to avoid messing up git history, see #8664 . Exceptions should be very rare: when rebase gives too many conflicts, simply squash all commits using the script shown in https://github.com/nim-lang/Nim/pull/9356

  1. Do not mix pure formatting changes (eg whitespace changes, nimpretty) or automated changes (eg nimfix) with other code changes: these should be in separate commits (and the merge on github should not squash these into 1).

Continuous Integration (CI)

  1. Continuous Integration is by default run on every push in a PR; this clogs the CI pipeline and affects other PR's; if you don't need it (eg for WIP or documentation only changes), add [ci skip] to your commit message title. This convention is supported by Appveyor and Travis
  1. Consider enabling CI (travis and appveyor) in your own Nim fork, and waiting for CI to be green in that fork (fixing bugs as needed) before opening your PR in original Nim repo, so as to reduce CI congestion. Same applies for updates on a PR: you can test commits on a separate private branch before updating the main PR.

Code reviews

  1. Whenever possible, use github's new 'Suggested change' in code reviews, which saves time explaining the change or applying it; see also https://forum.nim-lang.org/t/4317

Documentation Style

General Guidelines

  • Authors should document anything that is exported.
  • Within documentation, a period (.) should follow each sentence (or sentence fragment) in a comment block. The documentation may be limited to one sentence fragment, but if multiple sentences are within the documentation, each sentence after the first should be complete and in present tense.
  • Documentation is parsed as ReStructuredText (RST).
  • Inline code should be surrounded by double tick marks ("``````"). If you would like a character to immediately follow inline code (e.g., "int8``s are great!"), escape the following character with a backslash (``). The preceding is typed as ``` int8s are great!```.

Module-level documentation

Documentation of a module is placed at the top of the module itself. Each line of documentation begins with double hashes (##). Code samples are encouraged, and should follow the general RST syntax:

## The ``universe`` module computes the answer to life, the universe, and everything.
##
## .. code-block:: Nim
##  echo computeAnswerString() # "42"

Within this top-level comment, you can indicate the authorship and copyright of the code, which will be featured in the produced documentation.

## This is the best module ever. It provides answers to everything!
##
## :Author: Steve McQueen
## :Copyright: 1965
##

Leave a space between the last line of top-level documentation and the beginning of Nim code (the imports, etc.).

Procs, Templates, Macros, Converters, and Iterators

The documentation of a procedure should begin with a capital letter and should be in present tense. Variables referenced in the documentation should be surrounded by double tick marks (``````).

proc example1*(x: int) =
  ## Prints the value of ``x``.
  echo x

Whenever an example of usage would be helpful to the user, you should include one within the documentation in RST format as below.

proc addThree*(x, y, z: int8): int =
  ## Adds three ``int8`` values, treating them as unsigned and
  ## truncating the result.
  ##
  ## .. code-block:: nim
  ##  echo addThree(3, 125, 6) # -122
  result = x +% y +% z

The commands nim doc and nim doc2 will then correctly syntax highlight the Nim code within the documentation.

Types

Exported types should also be documented. This documentation can also contain code samples, but those are better placed with the functions to which they refer.

type
  NamedQueue*[T] = object ## Provides a linked data structure with names
                          ## throughout. It is named for convenience. I'm making
                          ## this comment long to show how you can, too.
    name*: string ## The name of the item
    val*: T ## Its value
    next*: ref NamedQueue[T] ## The next item in the queue

You have some flexibility when placing the documentation:

type
  NamedQueue*[T] = object
    ## Provides a linked data structure with names
    ## throughout. It is named for convenience. I'm making
    ## this comment long to show how you can, too.
    name*: string ## The name of the item
    val*: T ## Its value
    next*: ref NamedQueue[T] ## The next item in the queue

Make sure to place the documentation beside or within the object.

type
  ## This documentation disappears because it annotates the ``type`` keyword
  ## above, not ``NamedQueue``.
  NamedQueue*[T] = object
    name*: string ## This becomes the main documentation for the object, which
                  ## is not what we want.
    val*: T ## Its value
    next*: ref NamedQueue[T] ## The next item in the queue

Var, Let, and Const

When declaring module-wide constants and values, documentation is encouraged. The placement of doc comments is similar to the type sections.

const
  X* = 42 ## An awesome number.
  SpreadArray* = [
    [1,2,3],
    [2,3,1],
    [3,1,2],
  ] ## Doc comment for ``SpreadArray``.

Placement of comments in other areas is usually allowed, but will not become part of the documentation output and should therefore be prefaced by a single hash (#).

const
  BadMathVals* = [
    3.14, # pi
    2.72, # e
    0.58, # gamma
  ] ## A bunch of badly rounded values.

Nim supports Unicode in comments, so the above can be replaced with the following:

const
  BadMathVals* = [
    3.14, # π
    2.72, # e
    0.58, # γ
  ] ## A bunch of badly rounded values (including π!).