From d0dd5ea88719e9a85ab6e0708e5caee7caedd5e1 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Fri, 19 Oct 2018 12:03:25 -0700 Subject: [PATCH] [doc] add tips to doc/contributing.rst: git, code review, CI (#9429) --- doc/contributing.rst | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index fedabab11f..35cdd9f099 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -367,6 +367,39 @@ General commit rules eg: use ``git pull --rebase origin devel``. This is to avoid messing up git history, see `#8664 `_ . - Exceptions should be very rare. + 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 + + +5. 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 `_ + + +2. 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 .. include:: docstyle.rst + +