This document describes some best practices for collaborating on repositories. Following these practices makes it easier for contributors (new and old) to understand what is expected of them. It should be linked to in the README.md.
There are many good practices that this document does not cover. These include other members of the wider community reviewing pull requests (PRs) they are interested in, and maintainers encouraging and supporting people who open issues to make PRs to solve them. This document facilitates these other good practices by clarifying what can seem like a mysterious process to those who are unfamiliar with it.
This document is also only intended for community practices, it is not suitable for solo projects with one maintainer.
Interactions with people in the community must always follow the community standards, including in pull requests, issues, and discussions.
This page offers some further guidance on conventions that can be helpful when collaborating on projects. This is an expansion on the Collaborative Practices, with more details and extra guidance. Anything detailed here should be considered less important than the main Collaborative Practices.
Guidance on contributing PRsYou should usually open an issue about a bug or possible improvement before opening a PR with a solution.
PRs should do a single thing, so that they are easier to review.
PRs should add tests which cover the new or fixed functionality.
PRs that move code should not also change code, so that they are easier to review.
If only moving code, review for correctness is not required.
If only changing code, then the diff makes it clear what lines have changed.
PRs with large improvements to style should not also change functionality.
PRs introducing breaking changes should make this clear when opening the PR.
You should not push commits with commented-out tests.
@test_broken
macro.You should not squash down commits while review is still ongoing.
You should help review your PRs, even though you cannot approve your own PRs.
Review comments should be phrased as questions, as it shows you are open to new ideas.
Small review suggestions, such as typo fixes, should make use of the suggested change
feature.
Reviewers should continue acting as reviewers until the PR is merged.
Following the Collaborative Practices, when there are unreleased changes in the repository for an extended period of time, the version number in the Project.toml should be suffixed with -DEV
. This makes it clear that there are unreleased changes. Which is useful for many things, including quickly understanding why a bug is still occurring, and working out if a bugfix may need to be backported.
Some more details on the use of -DEV
.
After/during/before the PR making the first change of the release, the version number in the Project.toml should be changed to the intended release number should suffixed -DEV
.
-DEV
for a non-breaking change.
Follow-up PRs can then be made, which do not need to increment the version.
Once all the follow-up changes have been made, we can make a PR to drop the -DEV
suffix and make a new release once this PR is merged.
Note that locally, when using pkg”dev Foo...”
to install particular unreleased versions to an environment, Pkg ignores suffixes to the version number. Pkg treats 0.7.0-DEV
identically to 0.7.0
. This means you can update the [compat]
section of a group of packages and test them together.
5.4.0
, then we can still go back and release 5.3.1
.
Do not panic, these things sometimes slip through.
It is important to fix it as soon as possible, as otherwise people start using the breaking change, and reverting it later causes more problems (c.f. Murphy's law).
To fix it:
Once the change is reverted, you can take stock and decide what to do. There are generally 2 options:
Consider a package which is currently on v1.14.2. I made a PR to add a new feature and tagged release v1.15.0. The next evening, we get bug reports that the new feature actually broke lots of real uses.
Maybe I changed what I thought was an internal function, but one that was actually part of the public API; maybe I accidentally changed the return type, and that was something people depended on. Whatever it was, I broke it, and this was not caught in code review.
To fix it, I revert the change, and then tag release v1.15.1. Hopefully, I can also add a test to prevent that part of the API from being broken by mistake.
Now I look at my change again. If I can add the same functionality in a non-breaking way - for example, make a new internal function for my use - then I would do so and tag v1.15.2 or v1.16.0 depending on what had to change.Bu If I cannot make an equivalent non-breaking change, then I would have to make the breaking change and tag v2.0.0.
Accidental support for an unsupported dependencySay you were updating PackageA to support a new version of a dependency, PackageB. For example, you want PackageA v1.1.0 to support PackageB v0.5 and to discontinue supporting v0.4. But say you forgot to remove the compatibility for v0.4, which now no longer works, but other downstream packages that only use v0.4 are now pulling in PackageA v1.1.0 and getting errors.
Simply releasing a patch for PackageA (v1.1.1) that removes support for v0.4 won't work in this instance because downstream packages will continue to pull in v1.1.0. It might seem sufficient to just pin the downstream packages to use v1.0.0, but there may be a lot of them to fix, and you can't be certain you're aware of them all. It also does nothing to prevent new compatibility issues from arising in the future.
To fix this, you should still release a patch of PackageA (v1.1.1) that removes support for v0.4 of PackageB, but you should then mark v1.1.0 of PackageA as broken in the registry. To do this, simply make a PR to the registry, adding yanked = true
to the Version.toml
file under the version causing issues (in this case v1.1.0). This marks the release as broken and prevents it from being used by any package from then on.
Many of these guidelines can and should be enforced automatically.
The public API is defined as all exported symbols or symbols marked via the public
keyword (i.e. the full set of names from Docs.undocumented_names(module)
), plus any unexported symbols that are explicitly documented as part of the public API, for instance, documented as part of standard usage in the README or hosted documentation. Note that the latter part of the definition is a "deprecation path" because the public
keyword was only introduced in v1.11: it is expected in the future that the public API will be defined strictly as Docs.undocumented_names(module)
.
For macros, the public API is the documented set of behaviors and functionality.
Changes that are considered breakingpublic
keyword are public API. The full set of public names can be found by names(Package)
Docs.undocumented_names(module)
are still considered part of the public API, though it is highly recommended that any such names get documented ASAP.Everything on this list can, in theory, break users' code. See XKCD#1172. However, we consider changes to these things to be non-breaking from the perspective of package versioning.
Bugs: We may make backwards incompatible behavior changes if the current implementation is clearly broken, that is, if it contradicts the documentation or if a well-understood behavior is not properly implemented due to a bug.
Internal changes: Non-public API may be changed or removed.
Exception behavior:
Floating-point numerical details: The specific floating-point values may change at any time. Users should rely only on approximate accuracy, numerical stability, or statistical properties, not on the specific bits computed.
New exports: Adding a new export is never considered breaking. However, one should consider carefully before exporting a commonly used name that might clash with an existing name (especially, if clashing with Base
).
New supertypes:
A <: B
to A <: B <: C
or A <: C <: B
. This includes adding a supertype to something without one, i.e. with supertype Any
.Union
constant may be replaced by an abstract type that covers all elements of the union.Changes to the string representation: The output of print
/string
or show
/repr
on a type may change at any time. Users should not depend on the exact text, but rather on the meaning of the text. Changing the string representation often breaks downstream packages tests, because it is hard to write test-cases that depend only on meaning (though unit tests with mocking can be shielded from this kind of breaking).
(This guidance on non-breaking changes is inspired by https://www.tensorflow.org/guide/versions.)
Clarity on the Definition of Breaking in MacrosNote that the definition of breaking on macros is slightly larger than changes to its public API. This is due to the natural difficulty of documenting all parsable behaviors of macros and the difficulty of developing deprecation paths for changes to macro syntax. Thus in order to improve stability of the ecosystem with respect to macro changes, a more conservative definition of breaking is taken such that any change which does not have a backwards compatible alternative is considered breaking.
For example, if a macro had accidentally parsed @myequation 2x + 2 +
the same as @myequation 2x + 2
, it is not breaking to remove support for the trailing operator. If the trailing operator meant something which is not expressible in another way, and it is used in downstream packages, then it is considered a breaking change to remove this support.
public
keyword or export
._
internal identifier. For example, if MyPackage.f
is used, a new function MyPackage._f
should be created, used intnerally, and a deprecation notice telling users to not rely on MyPackage.f
should be added.As mentioned at the top, community repositories following ColPrac, should link to it in their README.md
. One way to do that is with a GitHub badge.
[](https://github.com/SciML/ColPrac)
Typically, ColPrac serves in places of a CONTRIBUTING.md
, having all the common guidance that you would otherwise put there. If your package has its own CONTRIBUTING.md
, then you should also link to ColPrac there, and indicate how the content of ColPrac relates to the CONTRIBUTING.md
. For example, by stating:
We follow the ColPrac guide for collaborative practices. New contributors should make sure to read that guide. Below are some additional practices we follow.
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4