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