contributing.rst 29 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641642643644645646647648649650651652653654655656657658659660661662663664665666667668669670671672673674675676677678679680681682683684685686687688689690691692693694695696697698699700701702703704705706707708709710711712713714715716717718719720721722723724725726727728729730731732733734735736737738739740741742743744745746747748749750751752753754755756757758759760761762763764765766767768769770771772773774775776777778779780781782783784785786787788
  1. ============
  2. Contributing
  3. ============
  4. .. default-role:: code
  5. .. include:: rstcommon.rst
  6. .. contents::
  7. Contributing happens via "Pull requests" (PR) on github. Every PR needs to be
  8. reviewed before it can be merged and the Continuous Integration should be green.
  9. The PR has to be approved by two core developers or by Araq.
  10. Writing tests
  11. =============
  12. There are 4 types of tests:
  13. 1. `runnableExamples` documentation comment tests, ran by `nim doc mymod.nim`:cmd:
  14. These end up in documentation and ensure documentation stays in sync with code.
  15. 2. separate test files, e.g.: ``tests/stdlib/tos.nim``.
  16. In nim repo, `testament`:cmd: (see below) runs all
  17. ``$nim/tests/*/t*.nim`` test files;
  18. for nimble packages, see https://github.com/nim-lang/nimble#tests.
  19. 3. (deprecated) tests in `when isMainModule:` block, ran by `nim r mymod.nim`:cmd:.
  20. `nimble test`:cmd: can run those in nimble packages when specified in a
  21. `task "test"`.
  22. 4. (not preferred) ``.. code-block:: nim`` RST snippets;
  23. these should only be used in rst sources,
  24. in nim sources `runnableExamples` should now always be preferred to those for
  25. several reasons (cleaner syntax, syntax highlights, batched testing, and
  26. parameter `rdoccmd` allows customization).
  27. Not all the tests follow the convention here, feel free to change the ones
  28. that don't. Always leave the code cleaner than you found it.
  29. Stdlib
  30. ------
  31. Each stdlib module (anything under ``lib/``, e.g. ``lib/pure/os.nim``) should
  32. preferably have a corresponding separate test file, e.g. ``tests/stdlib/tos.nim``.
  33. The old convention was to add a `when isMainModule:` block in the source file,
  34. which only gets executed when the tester is building the file.
  35. Each test should be in a separate `block:` statement, such that
  36. each has its own scope. Use boolean conditions and `doAssert` for the
  37. testing by itself, don't rely on echo statements or similar; in particular, avoid
  38. things like `echo "done"`. Don't use `unittest.suite` and `unittest.test`.
  39. Sample test:
  40. .. code-block:: nim
  41. block: # foo
  42. doAssert foo(1) == 10
  43. block: # bug #1234
  44. static: doAssert 1+1 == 2
  45. block: # bug #1235
  46. var seq2D = newSeqWith(4, newSeq[bool](2))
  47. seq2D[0][0] = true
  48. seq2D[1][0] = true
  49. seq2D[0][1] = true
  50. doAssert seq2D == @[@[true, true], @[true, false],
  51. @[false, false], @[false, false]]
  52. # doAssert with `not` can now be done as follows:
  53. doAssert not (1 == 2)
  54. Always refer to a GitHub issue using the following exact syntax: ``bug #1234`` as shown
  55. above, so that it's consistent and easier to search or for tooling. Some browser
  56. extensions (e.g. https://github.com/sindresorhus/refined-github) will even turn those
  57. in clickable links when it works.
  58. Rationale for using a separate test file instead of `when isMainModule:` block:
  59. * allows custom compiler flags or testing options (see details below)
  60. * faster CI since they can be joined in ``megatest`` (combined into a single test)
  61. * avoids making the parser do un-necessary work when a source file is merely imported
  62. * avoids mixing source and test code when reporting line of code statistics or code coverage
  63. Compiler
  64. --------
  65. The tests for the compiler use a testing tool called `testament`:cmd:. They are all
  66. located in ``tests/`` (e.g.: ``tests/destructor/tdestructor3.nim``).
  67. Each test has its own file. All test files are prefixed with `t`. If you want
  68. to create a file for import into another test only, use the prefix `m`.
  69. At the beginning of every test is the expected behavior of the test.
  70. Possible keys are:
  71. - `cmd`: A compilation command template e.g. `nim $target --threads:on $options $file`:cmd:
  72. - `output`: The expected output (stdout + stderr), most likely via `echo`
  73. - `exitcode`: Exit code of the test (via `exit(number)`)
  74. - `errormsg`: The expected compiler error message
  75. - `file`: The file the errormsg was produced at
  76. - `line`: The line the errormsg was produced at
  77. For a full spec, see here: ``testament/specs.nim``
  78. An example of a test:
  79. .. code-block:: nim
  80. discard """
  81. errormsg: "type mismatch: got (PTest)"
  82. """
  83. type
  84. PTest = ref object
  85. proc test(x: PTest, y: int) = nil
  86. var buf: PTest
  87. buf.test()
  88. Running tests
  89. =============
  90. You can run the tests with
  91. .. code-block:: cmd
  92. ./koch tests
  93. which will run a good subset of tests. Some tests may fail. If you
  94. only want to see the output of failing tests, go for
  95. ```cmd
  96. ./koch tests --failing all
  97. ```
  98. You can also run only a single category of tests. A category is a subdirectory
  99. in the ``tests/`` directory. There are a couple of special categories; for a
  100. list of these, see ``testament/categories.nim``, at the bottom.
  101. .. code:: cmd
  102. ./koch tests c lib # compiles / runs stdlib modules, including `isMainModule` tests
  103. ./koch tests c megatest # runs a set of tests that can be combined into 1
  104. To run a single test:
  105. .. code:: cmd
  106. ./koch test run <category>/<name> # e.g.: tuples/ttuples_issues
  107. ./koch test run tests/stdlib/tos.nim # can also provide relative path
  108. For reproducible tests (to reproduce an environment more similar to the one
  109. run by Continuous Integration on github actions/azure pipelines), you may want to disable your
  110. local configuration (e.g. in ``~/.config/nim/nim.cfg``) which may affect some
  111. tests; this can also be achieved by using
  112. `export XDG_CONFIG_HOME=pathtoAlternateConfig`:cmd: before running `./koch`:cmd:
  113. commands.
  114. Comparing tests
  115. ===============
  116. Test failures can be grepped using ``Failure:``.
  117. The tester can compare two test runs. First, you need to create a
  118. reference test. You'll also need to the commit id, because that's what
  119. the tester needs to know in order to compare the two.
  120. .. code:: cmd
  121. git checkout devel
  122. DEVEL_COMMIT=$(git rev-parse HEAD)
  123. ./koch tests
  124. Then switch over to your changes and run the tester again.
  125. .. code:: cmd
  126. git checkout your-changes
  127. ./koch tests
  128. Then you can ask the tester to create a ``testresults.html`` which will
  129. tell you if any new tests passed/failed.
  130. .. code:: cmd
  131. ./koch tests --print html $DEVEL_COMMIT
  132. Deprecation
  133. ===========
  134. Backward compatibility is important, so instead of a rename you need to deprecate
  135. the old name and introduce a new name:
  136. .. code-block:: nim
  137. # for routines (proc/template/macro/iterator) and types:
  138. proc oldProc(a: int, b: float): bool {.deprecated:
  139. "deprecated since v1.2.3; use `newImpl: string -> int` instead".} = discard
  140. # for (const/var/let/fields) the msg is not yet supported:
  141. const Foo {.deprecated.} = 1
  142. # for enum types, you can deprecate the type or some elements
  143. # (likewise with object types and their fields):
  144. type Bar {.deprecated.} = enum bar0, bar1
  145. type Barz = enum baz0, baz1 {.deprecated.}, baz2
  146. See also `Deprecated <manual.html#pragmas-deprecated-pragma>`_
  147. pragma in the manual.
  148. Documentation
  149. =============
  150. When contributing new procs, be sure to add documentation, especially if
  151. the proc is public. Even private procs benefit from documentation and can be
  152. viewed using `nim doc --docInternal foo.nim`:cmd:.
  153. Documentation begins on the line
  154. following the `proc` definition, and is prefixed by `##` on each line.
  155. Runnable code examples are also encouraged, to show typical behavior with a few
  156. test cases (typically 1 to 3 `assert` statements, depending on complexity).
  157. These `runnableExamples` are automatically run by `nim doc mymodule.nim`:cmd:
  158. as well as `testament`:cmd: and guarantee they stay in sync.
  159. .. code-block:: nim
  160. proc addBar*(a: string): string =
  161. ## Adds "Bar" to `a`.
  162. runnableExamples:
  163. assert "baz".addBar == "bazBar"
  164. result = a & "Bar"
  165. See `parentDir <os.html#parentDir,string>`_ example.
  166. The RestructuredText Nim uses has a special syntax for including code snippets
  167. embedded in documentation; these are not run by `nim doc`:cmd: and therefore are
  168. not guaranteed to stay in sync, so `runnableExamples` is almost always preferred:
  169. .. code-block:: nim
  170. proc someProc*(): string =
  171. ## Returns "something"
  172. ##
  173. ## .. code-block::
  174. ## echo someProc() # "something"
  175. result = "something" # single-hash comments do not produce documentation
  176. The ``.. code-block:: nim`` followed by a newline and an indentation instructs the
  177. `nim doc`:cmd: command to produce syntax-highlighted example code with the
  178. documentation (``.. code-block::`` is sufficient from inside a nim module).
  179. When forward declaration is used, the documentation should be included with the
  180. first appearance of the proc.
  181. .. code-block:: nim
  182. proc hello*(): string
  183. ## Put documentation here
  184. proc nothing() = discard
  185. proc hello*(): string =
  186. ## ignore this
  187. echo "hello"
  188. The preferred documentation style is to begin with a capital letter and use
  189. the third-person singular. That is, between:
  190. .. code-block:: nim
  191. proc hello*(): string =
  192. ## Returns "hello"
  193. result = "hello"
  194. or
  195. .. code-block:: nim
  196. proc hello*(): string =
  197. ## say hello
  198. result = "hello"
  199. the first is preferred.
  200. When you specify an *RST role* (highlighting/interpretation marker) do it
  201. in the postfix form for uniformity, that is after \`text in backticks\`.
  202. For example an ``:idx:`` role for referencing a topic ("SQLite" in the
  203. example below) from `Nim Index`_ can be used in doc comment this way:
  204. .. code-block:: nim
  205. ## A higher level `SQLite`:idx: database wrapper.
  206. .. _`Nim Index`: https://nim-lang.org/docs/theindex.html
  207. Inline monospaced text can be input using \`single backticks\` or
  208. \`\`double backticks\`\`. The former are syntactically highlighted,
  209. the latter are not.
  210. To avoid accidental highlighting follow this rule in ``*.nim`` files:
  211. * use single backticks for fragments of code in Nim and other
  212. programming languages, including identifiers, in ``*.nim`` files.
  213. For languages other than Nim add a role after final backtick,
  214. e.g. for C++ inline highlighting::
  215. `#include <stdio.h>`:cpp:
  216. For a currently unsupported language add the `:code:` role,
  217. like for SQL in this example::
  218. `SELECT * FROM <table_name>;`:code:
  219. Highlight shell commands by ``:cmd:`` role; for command line options use
  220. ``:option:`` role, e.g.: \`--docInternal\`:option:.
  221. * prefer double backticks otherwise:
  222. * for file names: \`\`os.nim\`\`
  223. * for fragments of strings **not** enclosed by `"` and `"` and not
  224. related to code, e.g. text of compiler messages
  225. * also when code ends with a standalone ``\`` (otherwise a combination of
  226. ``\`` and a final \` would get escaped)
  227. .. Note:: ``*.rst`` files have ``:literal:`` as their default role.
  228. So for them the rule above is only applicable if the ``:nim:`` role
  229. is set up manually as the default [*]_::
  230. .. role:: nim(code)
  231. :language: nim
  232. .. default-role:: nim
  233. The first 2 lines are for other RST implementations,
  234. including Github one.
  235. .. [*] this is fulfilled when ``doc/rstcommon.rst`` is included.
  236. Best practices
  237. ==============
  238. Note: these are general guidelines, not hard rules; there are always exceptions.
  239. Code reviews can just point to a specific section here to save time and
  240. propagate best practices.
  241. .. _define_needs_prefix:
  242. New `defined(foo)` symbols need to be prefixed by the nimble package name, or
  243. by `nim` for symbols in nim sources (e.g. compiler, standard library). This is
  244. to avoid name conflicts across packages.
  245. .. code-block:: nim
  246. # if in nim sources
  247. when defined(allocStats): discard # bad, can cause conflicts
  248. when defined(nimAllocStats): discard # preferred
  249. # if in a package `cligen`:
  250. when defined(debug): discard # bad, can cause conflicts
  251. when defined(cligenDebug): discard # preferred
  252. .. _noimplicitbool:
  253. Take advantage of no implicit bool conversion
  254. .. code-block:: nim
  255. doAssert isValid() == true
  256. doAssert isValid() # preferred
  257. .. _design_for_mcs:
  258. Design with method call syntax chaining in mind
  259. .. code-block:: nim
  260. proc foo(cond: bool, lines: seq[string]) # bad
  261. proc foo(lines: seq[string], cond: bool) # preferred
  262. # can be called as: `getLines().foo(false)`
  263. .. _avoid_quit:
  264. Use exceptions (including `assert` / `doAssert`) instead of `quit`
  265. rationale: https://forum.nim-lang.org/t/4089
  266. .. code-block:: nim
  267. quit() # bad in almost all cases
  268. doAssert() # preferred
  269. .. _tests_use_doAssert:
  270. Use `doAssert` (or `unittest.check`, `unittest.require`), not `assert` in all
  271. tests so they'll be enabled even with `--assertions:off`:option:.
  272. .. code-block:: nim
  273. block: # foo
  274. assert foo() # bad
  275. doAssert foo() # preferred
  276. .. _runnableExamples_use_assert:
  277. An exception to the above rule is `runnableExamples` and ``code-block`` rst blocks
  278. intended to be used as `runnableExamples`, which for brevity use `assert`
  279. instead of `doAssert`. Note that `nim doc -d:danger main`:cmd: won't pass `-d:danger`:option: to the
  280. `runnableExamples`, but `nim doc --doccmd:-d:danger main`:cmd: would, and so would the
  281. second example below:
  282. .. code-block:: nim
  283. runnableExamples:
  284. doAssert foo() # bad
  285. assert foo() # preferred
  286. runnableExamples("-d:danger"):
  287. doAssert foo() # `assert` would be disabled here, so `doAssert` makes more sense
  288. .. _delegate_printing:
  289. Delegate printing to caller: return `string` instead of calling `echo`
  290. rationale: it's more flexible (e.g. allows the caller to call custom printing,
  291. including prepending location info, writing to log files, etc).
  292. .. code-block:: nim
  293. proc foo() = echo "bar" # bad
  294. proc foo(): string = "bar" # preferred (usually)
  295. .. _use_Option:
  296. [Ongoing debate] Consider using Option instead of return bool + var argument,
  297. unless stack allocation is needed (e.g. for efficiency).
  298. .. code-block:: nim
  299. proc foo(a: var Bar): bool
  300. proc foo(): Option[Bar]
  301. .. _use_doAssert_not_echo:
  302. Tests (including in testament) should always prefer assertions over `echo`,
  303. except when that's not possible. It's more precise, easier for readers and
  304. maintainers to where expected values refer to. See for example
  305. https://github.com/nim-lang/Nim/pull/9335 and https://forum.nim-lang.org/t/4089
  306. .. code-block:: nim
  307. echo foo() # adds a line for testament in `output:` block inside `discard`.
  308. doAssert foo() == [1, 2] # preferred, except when not possible to do so.
  309. The `git`:cmd: stuff
  310. ====================
  311. General commit rules
  312. --------------------
  313. 1. Important, critical bugfixes that have a tiny chance of breaking
  314. somebody's code should be backported to the latest stable release
  315. branch (currently 1.4.x) and maybe also all the way back to the 1.0.x branch.
  316. The commit message should contain the tag ``[backport]`` for "backport to the latest
  317. stable release" and the tag ``[backport:$VERSION]`` for backporting back to the
  318. given $VERSION (and all newer releases).
  319. 2. If you introduce changes which affect backward compatibility,
  320. make breaking changes, or have PR which is tagged as ``[feature]``,
  321. the changes should be mentioned in `the changelog
  322. <https://github.com/nim-lang/Nim/blob/devel/changelog.md>`_.
  323. 3. All changes introduced by the commit (diff lines) must be related to the
  324. subject of the commit.
  325. If you change something unrelated to the subject parts of the file, because
  326. your editor reformatted automatically the code or whatever different reason,
  327. this should be excluded from the commit.
  328. *Tip:* Never commit everything as is using `git commit -a`:cmd:, but review
  329. carefully your changes with `git add -p`:cmd:.
  330. 4. Changes should not introduce any trailing whitespace.
  331. Always check your changes for whitespace errors using `git diff --check`:cmd:
  332. or add the following ``pre-commit`` hook:
  333. .. code:: cmd
  334. #!/bin/sh
  335. git diff --check --cached || exit $?
  336. 5. Describe your commit and use your common sense.
  337. Example commit message::
  338. Fixes #123; refs #124
  339. indicates that issue ``#123`` is completely fixed (GitHub may automatically
  340. close it when the PR is committed), wheres issue ``#124`` is referenced
  341. (e.g.: partially fixed) and won't close the issue when committed.
  342. 6. PR body (not just PR title) should contain references to fixed/referenced github
  343. issues, e.g.: ``fix #123`` or ``refs #123``. This is so that you get proper cross
  344. referencing from linked issue to the PR (github won't make those links with just
  345. PR title, and commit messages aren't always sufficient to ensure that, e.g.
  346. can't be changed after a PR is merged).
  347. 7. Commits should be always be rebased against devel (so a fast forward
  348. merge can happen)
  349. e.g.: use `git pull --rebase origin devel`:cmd:. This is to avoid messing up
  350. git history.
  351. Exceptions should be very rare: when rebase gives too many conflicts, simply
  352. squash all commits using the script shown in
  353. https://github.com/nim-lang/Nim/pull/9356
  354. 8. Do not mix pure formatting changes (e.g. whitespace changes, nimpretty) or
  355. automated changes (e.g. nimfix) with other code changes: these should be in
  356. separate commits (and the merge on GitHub should not squash these into 1).
  357. Continuous Integration (CI)
  358. ---------------------------
  359. 1. Continuous Integration is by default run on every push in a PR; this clogs
  360. the CI pipeline and affects other PR's; if you don't need it (e.g. for WIP or
  361. documentation only changes), add ``[skip ci]`` to your commit message title.
  362. This convention is supported by our github actions pipelines and our azure pipeline
  363. (using custom logic, which should complete in < 1mn) as well as our former other pipelines:
  364. `Appveyor <https://www.appveyor.com/docs/how-to/filtering-commits/#skip-directive-in-commit-message>`_
  365. and `Travis <https://docs.travis-ci.com/user/customizing-the-build/#skipping-a-build>`_.
  366. 2. Consider enabling CI (azure, GitHub actions and builds.sr.ht) in your own Nim fork, and
  367. waiting for CI to be green in that fork (fixing bugs as needed) before
  368. opening your PR in the original Nim repo, so as to reduce CI congestion. Same
  369. applies for updates on a PR: you can test commits on a separate private
  370. branch before updating the main PR.
  371. Debugging CI failures, flaky tests, etc
  372. ---------------------------------------
  373. 1. First check the CI logs and search for `FAIL` to find why CI failed; if the
  374. failure seems related to your PR, try to fix the code instead of restarting CI.
  375. 2. If CI failure seems unrelated to your PR, it could be caused by a flaky test.
  376. File a bug for it if it isn't already reported. A PR push (or opening/closing PR)
  377. will re-trigger all CI jobs (even successful ones, which can be wasteful). Instead,
  378. follow these instructions to only restart the jobs that failed:
  379. * Azure: if on your own fork, it's possible from inside azure console
  380. (e.g. ``dev.azure.com/username/username/_build/results?buildId=1430&view=results``) via ``rerun failed jobs`` on top.
  381. If either on you own fork or in Nim repo, it's possible from inside GitHub UI
  382. under checks tab, see https://github.com/timotheecour/Nim/issues/211#issuecomment-629751569
  383. * GitHub actions: under "Checks" tab, click "Re-run jobs" in the right.
  384. * builds.sr.ht: create a sourcehut account so you can restart a PR job as illustrated.
  385. builds.sr.ht also allows you to ssh to a CI machine which can help a lot for debugging
  386. issues, see docs in https://man.sr.ht/builds.sr.ht/build-ssh.md and
  387. https://drewdevault.com/2019/08/19/Introducing-shell-access-for-builds.html; see
  388. https://man.sr.ht/tutorials/set-up-account-and-git.md to generate and upload ssh keys.
  389. Code reviews
  390. ------------
  391. 1. Whenever possible, use GitHub's new 'Suggested change' in code reviews, which
  392. saves time explaining the change or applying it; see also
  393. https://forum.nim-lang.org/t/4317
  394. 2. When reviewing large diffs that may involve code moving around, GitHub's interface
  395. doesn't help much as it doesn't highlight moves. Instead, you can use something
  396. like this, see visual results `here <https://github.com/nim-lang/Nim/pull/10431#issuecomment-456968196>`_:
  397. .. code:: cmd
  398. git fetch origin pull/10431/head && git checkout FETCH_HEAD
  399. git diff --color-moved-ws=allow-indentation-change --color-moved=blocks HEAD^
  400. 3. In addition, you can view GitHub-like diffs locally to identify what was changed
  401. within a code block using `diff-highlight`:cmd: or `diff-so-fancy`:cmd:, e.g.:
  402. ::
  403. # put this in ~/.gitconfig:
  404. [core]
  405. pager = "diff-so-fancy | less -R" # or: use: `diff-highlight`
  406. .. include:: docstyle.rst
  407. Evolving the stdlib
  408. ===================
  409. As outlined in https://github.com/nim-lang/RFCs/issues/173 there are a couple
  410. of guidelines about what should go into the stdlib, what should be added and
  411. what eventually should be removed.
  412. What the compiler itself needs must be part of the stdlib
  413. ---------------------------------------------------------
  414. Maybe in the future the compiler itself can depend on Nimble packages but for
  415. the time being, we strive to have zero dependencies in the compiler as the
  416. compiler is the root of the bootstrapping process and is also used to build
  417. Nimble.
  418. Vocabulary types must be part of the stdlib
  419. -------------------------------------------
  420. These are types most packages need to agree on for better interoperability,
  421. for example `Option[T]`. This rule also covers the existing collections like
  422. `Table`, `CountTable` etc. "Sorted" containers based on a tree-like data
  423. structure are still missing and should be added.
  424. Time handling, especially the `Time` type are also covered by this rule.
  425. Existing, battle-tested modules stay
  426. ------------------------------------
  427. Reason: There is no benefit in moving them around just to fullfill some design
  428. fashion as in "Nim's core MUST BE SMALL". If you don't like an existing module,
  429. don't import it. If a compilation target (e.g. JS) cannot support a module,
  430. document this limitation.
  431. This covers modules like `os`, `osproc`, `strscans`, `strutils`,
  432. `strformat`, etc.
  433. Syntactic helpers can start as experimental stdlib modules
  434. ----------------------------------------------------------
  435. Reason: Generally speaking as external dependencies they are not exposed
  436. to enough users so that we can see if the shortcuts provide enough benefit
  437. or not. Many programmers avoid external dependencies, even moreso for
  438. "tiny syntactic improvements". However, this is only true for really good
  439. syntactic improvements that have the potential to clean up other parts of
  440. the Nim library substantially. If in doubt, new stdlib modules should start
  441. as external, successful Nimble packages.
  442. Other new stdlib modules do not start as stdlib modules
  443. -------------------------------------------------------
  444. As we strive for higher quality everywhere, it's easier to adopt existing,
  445. battle-tested modules eventually rather than creating modules from scratch.
  446. Little additions are acceptable
  447. -------------------------------
  448. As long as they are documented and tested well, adding little helpers
  449. to existing modules is acceptable. For two reasons:
  450. 1. It makes Nim easier to learn and use in the long run.
  451. ("Why does sequtils lack a `countIt`?
  452. Because version 1.0 happens to have lacked it? Silly...")
  453. 2. To encourage contributions. Contributors often start with PRs that
  454. add simple things and then they stay and also fix bugs. Nim is an
  455. open source project and lives from people's contributions and involvement.
  456. Newly introduced issues have to be balanced against motivating new people. We know where
  457. to find perfectly designed pieces of software that have no bugs -- these are the systems
  458. that nobody uses.
  459. Conventions
  460. -----------
  461. 1. New stdlib modules should go under ``Nim/lib/std/``. The rationale is to
  462. require users to import via `import std/foo` instead of `import foo`,
  463. which would cause potential conflicts with nimble packages.
  464. Note that this still applies for new modules in existing logical
  465. directories, e.g.: use ``lib/std/collections/foo.nim``,
  466. not ``lib/pure/collections/foo.nim``.
  467. 2. New module names should prefer plural form whenever possible, e.g.:
  468. ``std/sums.nim`` instead of ``std/sum.nim``. In particular, this reduces
  469. chances of conflicts between module name and the symbols it defines.
  470. Furthermore, module names should use `snake_case` and not use capital
  471. letters, which cause issues when going from an OS without case
  472. sensitivity to an OS with it.
  473. Breaking Changes
  474. ================
  475. Introducing breaking changes, no matter how well intentioned,
  476. creates long-term problems for the community, in particular those looking to promote
  477. reusable Nim code in libraries: In the Nim distribution, critical security and bugfixes,
  478. language changes and community improvements are bundled in a single distribution - it is
  479. difficult to make partial upgrades with only benign changes. When one library depends on
  480. a legacy behavior, it can no longer be used together with another library that does not,
  481. breaking all downstream applications - the standard library is unique in that it sits at
  482. the root of **all** dependency trees.
  483. There is a big difference between compile-time breaking changes and run-time breaking
  484. changes.
  485. Run-time breaking changes
  486. -------------------------
  487. Run-time breaking changes are to be avoided at almost all costs: Nim is used for
  488. mission critical applications which depend on behaviours that
  489. are not covered by the test suite. As such, it's important that changes to the
  490. *stable* parts of the standard library are made avoiding changing the existing
  491. behaviors, even when the test suite continues to pass.
  492. Examples of run-time breaking changes:
  493. - Raising exceptions of a new type, compared to what's currently being raised.
  494. - Adding unconstrained or poorly constrained generic procs or macros
  495. ("hash now works for all `ref T`"): This may cause code to behave differently
  496. depending only on which modules are imported - common examples include `==` and `hash`.
  497. - Changing behavior of existing functions like:
  498. * "Nim's path handling procs like `getXDir` now consistently lack the trailing slash"
  499. * "Nim's strformat implementation is now more consistent with Python"
  500. Instead write new code that explicitly announces the feature you think we announced but
  501. didn't. For example, `strformat` does not say "it's compatible with Python", it
  502. says "inspired by Python's f-strings". This new code can be submitted to the stdlib
  503. and the old code can be deprecated or it can be published as a Nimble package.
  504. Sometimes, a run-time breaking change is most desirable: For example, a string
  505. representation of a floating point number that "roundtrips" is much better than
  506. a string represenation that doesn't. These run-time breaking changes must start in the
  507. state "opt-in" via a new `-d:nimPreviewX` or command line flag and then should become
  508. the new default later, in follow-up versions. This way users can track
  509. regressions more easily. ("git bisect" is not an acceptable alternative, that's for
  510. Nim compiler developers, not for Nim users.)
  511. Above all else, additive approaches that don't change existing behaviors
  512. should be preferred.
  513. Compile-time breaking changes
  514. -----------------------------
  515. Compile-time breaking changes are usually easier to handle, but for large code bases
  516. it can also be much work and it can hinder the adoption of a new Nim release.
  517. Additive approaches are to be preferred here as well.
  518. Examples of compile-time breaking changes include (but are not limited to):
  519. * Renaming functions and modules, or moving things. Instead of a direct rename,
  520. deprecate the old name and introduce a new one.
  521. * Renaming the parameter names: Thanks to Nim's "named parameter" calling syntax
  522. like `f(x = 0, y = 1)` this is a breaking change. Instead live with the existing
  523. parameter names.
  524. * Adding an enum value to an existing enum. Nim's exhaustive case statements stop
  525. compiling after such a change. Instead consider to introduce new `bool`
  526. fields/parameters. This can be impractical though, so we use good judgement
  527. and our list of "important packages" to see if it doesn't break too much code
  528. out there in practice.
  529. * Adding a new proc to an existing stdlib module. However, for aesthetic reasons
  530. this is often preferred over introducing a new module with just a single proc
  531. inside. We use good judgement and our list of "important packages" to see if
  532. it doesn't break too much code out there in practice. The new procs need to
  533. be annotated with a `.since` annotation.
  534. Compiler/language spec bugfixes
  535. -------------------------------
  536. This can even be applied to compiler "bugfixes": If the compiler should have been
  537. "pickier" in its handling of `typedesc`, instead of "fixing typedesc handling bugs",
  538. consider the following solution:
  539. - Spec out how `typedesc` should really work and also spec out the cases where it
  540. should not be allowed!
  541. - Deprecate `typedesc` and name the new metatype something new like `typeArg`.
  542. - Implement the spec.
  543. Non-breaking changes
  544. --------------------
  545. Examples of changes that are considered non-breaking (or acceptable breaking changes) include:
  546. * Creating a new module.
  547. * Adding an overload to an already overloaded proc.
  548. * Adding new default parameters to an existing proc. It is assumed that you do not
  549. use Nim's stdlib procs's addresses (that you don't use them as first class entities).
  550. * Changing the calling convention from `nimcall` to `inline`
  551. (but first RFC https://github.com/nim-lang/RFCs/issues/396 needs to be implemented).
  552. * Changing the behavior from "crashing" into some other, well documented result (including
  553. raising a Defect, but not raising an exception that does not inherit from Defect).
  554. * Adding new fields to an existing object.
  555. Nim's introspection facilities imply that strictly speaking almost every addition can
  556. break somebody's code. It is impractical to care about these cases, a change that only
  557. affects introspection is not considered to be a breaking change.