diff options
Diffstat (limited to 'doc/contributing.md')
| -rw-r--r-- | doc/contributing.md | 798 |
1 files changed, 798 insertions, 0 deletions
diff --git a/doc/contributing.md b/doc/contributing.md new file mode 100644 index 000000000..420c1438e --- /dev/null +++ b/doc/contributing.md @@ -0,0 +1,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 [^1]: + + .. role:: nim(code) + :language: nim + .. default-role:: nim + + The first 2 lines are for other RST implementations, + including Github one. + + [^1]: 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. |