aboutsummaryrefslogtreecommitdiff
path: root/CONTRIBUTING.md
diff options
context:
space:
mode:
Diffstat (limited to 'CONTRIBUTING.md')
-rw-r--r--CONTRIBUTING.md95
1 files changed, 79 insertions, 16 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 54ecfb0..10d1149 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -65,10 +65,6 @@ and setup. Subsequent runs will be faster, but there are many tests, and some of
them are slow. If you're working on a particular area of code, you can run just
the tests in those directories instead, which can speed up your edit-run cycle.
-Note that there are tests to verify generated documentation is correct -- if
-you're modifying the signature of a public function, these tests will likely
-fail and you'll need to [regenerate the api docs](#documentation).
-
## Formatting
Starlark files should be formatted by
@@ -128,7 +124,9 @@ BREAKING CHANGE: <summary>
```
Where `(scope)` is optional, and `!` is only required if there is a breaking change.
-If a breaking change is introduced, then `BREAKING CHANGE:` is required.
+If a breaking change is introduced, then `BREAKING CHANGE:` is required; see
+the [Breaking Changes](#breaking-changes) section for how to introduce breaking
+changes.
Common `type`s:
@@ -146,18 +144,11 @@ For the full details of types, see
## Generated files
Some checked-in files are generated and need to be updated when a new PR is
-merged.
-
-### Documentation
+merged:
-To regenerate the content under the `docs/` directory, run this command:
-
-```shell
-bazel run //docs:update
-```
-
-This needs to be done whenever the docstrings in the corresponding .bzl files
-are changed; a test failure will remind you to run this command when needed.
+* **requirements lock files**: These are usually generated by a
+ `compile_pip_requirements` update target, which is usually in the same directory.
+ e.g. `bazel run //docs/sphinx:requirements.update`
## Core rules
@@ -184,6 +175,78 @@ Issues should be triaged as follows:
functionality, should also be filed in this repository but without the
`core-rules` label.
+## Breaking Changes
+
+Breaking changes are generally permitted, but we follow a 3-step process for
+introducing them. The intent behind this process is to balance the difficulty of
+version upgrades for users, maintaining multiple code paths, and being able to
+introduce modern functionality.
+
+The general process is:
+
+1. In version `N`, introduce the new behavior, but it must be disabled by
+ default. Users can opt into the new functionality when they upgrade to
+ version `N`, which lets them try it and verify functionality.
+2. In version `N+1`, the new behavior can be enabled by default. Users can
+ opt out if necessary, but doing so causes a warning to be issued.
+3. In version `N+2`, the new behavior is always enabled and cannot be opted out
+ of. The API for the control mechanism can be removed in this release.
+
+Note that the `+1` and `+2` releases are just examples; the steps are not
+required to happen in immedially subsequent releases.
+
+
+### How to control breaking changes
+
+The details of the control mechanism will depend on the situation. Below is
+a summary of some different options.
+
+* Environment variables are best for repository rule behavior. Environment
+ variables can be propagated to rules and macros using the generated
+ `@rules_python_internal//:config.bzl` file.
+* Attributes are applicable to macros and regular rules, especially when the
+ behavior is likely to vary on a per-target basis.
+* [User defined build settings](https://bazel.build/extending/config#user-defined-build-settings)
+ (aka custom build flags) are applicable for rules when the behavior change
+ generally wouldn't vary on a per-target basis. They also have the benefit that
+ an entire code base can have them easily enabled by a bazel command line flag.
+* Allowlists allow a project to centrally control if something is
+ enabled/disabled. Under the hood, they are basically a specialized custom
+ build flag.
+
+Note that attributes and flags can seamlessly interoperate by having the default
+controlled by a flag, and an attribute can override the flag setting. This
+allows a project to enable the new behavior by default while they work to fix
+problematic cases to prepare for the next upgrade.
+
+### What is considered a breaking change?
+
+Precisely defining what constitutes a breaking change is hard because it's
+easy for _someone, somewhere_ to depend on _some_ observable behavior, despite
+our best efforts to thoroughly document what is or isn't supported and hiding
+any internal details.
+
+In general, something is considered a breaking change when it changes the
+direct behavior of a supported public API. Simply being able to observe a
+behavior change doesn't necessarily mean it's a breaking change.
+
+Long standing undocumented behavior is a large grey area and really depends on
+how load-bearing it has become and what sort of reasonable expectation of
+behavior there is.
+
+Here's some examples of what would or wouldn't be considered a breaking change.
+
+Breaking changes:
+ * Renaming an function argument for public functions.
+ * Enforcing stricter validation than was previously required when there's a
+ sensible reason users would run afoul of it.
+ * Changing the name of a public rule.
+
+Not breaking changes:
+ * Upgrading dependencies
+ * Changing internal details, such as renaming an internal file.
+ * Changing a rule to a macro.
+
## FAQ
### Installation errors when during `git commit`