make contributing.rst more up to date; reference it in readme.md to make it more discoverable (#9302)

2 files changed, 94 insertions, 27 deletions

diff --git a/doc/contributing.rst b/doc/contributing.rst
index ee97f6dc8..814b1af28 100644
--- a/doc/contributing.rst
+++ b/doc/contributing.rst
@@ -1,14 +1,24 @@
 Writing tests
 =============
 
-Not all the tests follow this scheme, feel free to change the ones
+There are 3 types of tests:
+
+1. ``runnableExamples`` documentation comment tests, ran by ``nim doc mymod.nim``
+   These end up in documentation and ensure documentation stays in sync with code.
+
+2. tests in `when isMainModule:` block, ran by ``nim c mymod.nim``
+   ``nimble test`` also typially runs these in external nimble packages.
+
+3. testament tests, eg: tests/stdlib/tospaths.nim (only used for Nim repo).
+
+Not all the tests follow the convention here, 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:``
+If you change the stdlib (anything under ``lib/``, eg ``lib/pure/ospaths.nim``),
+put a test in the file you changed. Add the tests under a ``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
@@ -26,24 +36,32 @@ Sample test:
       seq2D[0][1] = true
       doAssert seq2D == @[@[true, true], @[true, false],
                           @[false, false], @[false, false]]
+      # doAssert with `not` can be done as follows:
+      doAssert: not 1 == 2
+
+Newer tests tend to be run via ``testament`` rather than via ``when isMainModule:``,
+eg ``tests/stdlib/tospaths.nim``; this allows additional features such as custom
+compiler flags; for more details see below.
 
 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``.
+The tests for the compiler use a testing tool called ``testament``. They are all
+located in ``tests/`` (eg: ``tests/destructor/tdestructor3.nim``).
+Each test has its own file. 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.
+At the beginning of every test is the expected behavior 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
+- file: The file the errormsg was produced at
 - line: The line the errormsg was produced at
 
+For a full spec, see here: ``tests/testament/specs.nim``
+
 An example for a test:
 
 .. code-block:: nim
@@ -84,11 +102,21 @@ list of these, see ``tests/testament/categories.nim``, at the bottom.
 
   ./koch tests c lib
 
+
+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.
+Because some tests fail in the current ``devel`` branch, not every failure
+after your change is necessarily caused by your changes. Some tests are
+flaky and will fail on occasion; these are typically bugs that should be fixed.
+Test failures can be grepped using ``Failure:``.
 
 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
@@ -118,26 +146,34 @@ tell you if any new tests passed/failed.
 Deprecation
 ===========
 
-Backward compatibility is important, so if you are renaming a proc or
-a type, you can use
-
+Backward compatibility is important, so if you are renaming a routine
+(proc/template/macro/iterator) or a type you can use:
 
 .. code-block:: nim
 
   {.deprecated: [oldName: new_name].}
 
-Or you can simply use
+This form may enable future automated tooling to upgrade code (eg nimfix, which
+was used in the past for nimrod -> nim migration).
+
+Besides pure renamings (eg when parameters change) you can mark a symbol as
+deprecated using the following:
 
 .. code-block:: nim
 
-  proc oldProc() {.deprecated.}
+  # for routines (proc/template/macro/iterator) and types:
+  proc oldProc() {.deprecated: "use `newImpl: string -> int` instead".} = ...
 
-to mark a symbol as deprecated. Works for procs/types/vars/consts,
-etc. Note that currently the ``deprecated`` statement does not work well with
+  # for (const/var/let) the msg is not yet supported:
+  const Foo {.deprecated.}  = 1
+
+  # for enum types ``deprecated`` is not yet supported.
+
+Note that currently the ``deprecated`` statement does not work well with
 overloading so for routines the latter variant is better.
 
 
-`Deprecated <https://nim-lang.org/docs/manual.html#pragmas-deprecated-pragma>`_
+See also `Deprecated <https://nim-lang.org/docs/manual.html#pragmas-deprecated-pragma>`_
 pragma in the manual.
 
 
@@ -145,11 +181,28 @@ Documentation
 =============
 
 When contributing new procedures, be sure to add documentation, especially if
-the procedure is exported from the module. Documentation begins on the line
+the procedure is public. 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.
+Runnable code examples are also encouraged, to show typical behavior with a few
+test cases (typically 1 to 3 ``doAssert`` statements, depending on complexity).
+These ``runnableExamples`` are automatically run by ``nim doc mymodule.nim``
+as well as ``testament`` and guarantee they stay in sync.
+
+.. code-block:: nim
+  proc addBar*(a: string): string =
+    ## Adds "Bar" to ``a``.
+    runnableExamples:
+      doAssert "baz".addBar == "bazBar"
+     
+     result = a & "Bar"
+
+See `parentDir <https://nim-lang.github.io/Nim/ospaths.html#parentDir%2Cstring>`_
+example.
+
+The RestructuredText Nim uses has a special syntax for including code snippets
+embedded in documentation; these are not run by ``nim doc`` and therefore are
+not guaranteed to stay in sync, so ``runnableExamples`` is usually preferred:
 
 .. code-block:: nim
 
@@ -162,8 +215,8 @@ syntax for including examples.
     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.
+``nim doc`` command 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.
@@ -174,7 +227,7 @@ first appearance of the proc.
     ## Put documentation here
   proc nothing() = discard
   proc hello*(): string =
-    ## Ignore this
+    ## ignore this
     echo "hello"
 
 The preferred documentation style is to begin with a capital letter and use
@@ -204,7 +257,7 @@ General commit rules
 1. 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
+   If you change something 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.
 
@@ -223,4 +276,17 @@ General commit rules
 
 3. 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.
+
+4. 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 <https://github.com/nim-lang/Nim/issues/8664>`_ .
+   Exceptions should be very rare.
+
 .. include:: docstyle.rst
diff --git a/readme.md b/readme.md
index 3d0b7c4ce..7b81d0790 100644
--- a/readme.md
+++ b/readme.md
@@ -102,7 +102,7 @@ within the [doc/koch.rst](doc/koch.rst) file.
 
 ## Contributors
 
-This project exists thanks to all the people who contribute. [Read on to find out how to contribute](#contributing).
+This project exists thanks to all the people who contribute.
 <a href="https://github.com/nim-lang/Nim/graphs/contributors"><img src="https://opencollective.com/Nim/contributors.svg?width=890" /></a>
 
 ## Contributing
@@ -111,6 +111,7 @@ This project exists thanks to all the people who contribute. [Read on to find ou
 [![Donate Bitcoins][badge-nim-bitcoin]][nim-bitcoin]
 [![Open Source Helpers](https://www.codetriage.com/nim-lang/nim/badges/users.svg)](https://www.codetriage.com/nim-lang/nim)
 
+See [detailed contributing guidelines](https://nim-lang.github.io/Nim/contributing.html).
 We welcome all contributions to Nim regardless of how small or large
 they are. Everything from spelling fixes to new modules to be included in the
 standard library are welcomed and appreciated. Before you start contributing,