diff options
Diffstat (limited to 'CONTRIBUTING.md')
-rw-r--r-- | CONTRIBUTING.md | 95 |
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` |