-
Notifications
You must be signed in to change notification settings - Fork 2.2k
chore: Define backport criteria #22766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,6 +59,86 @@ To prepare for a new release series, maintainers: | |||||||||||||||||||||||||||||||||
| - Create release candidate artifacts from the release branch | ||||||||||||||||||||||||||||||||||
| - After approval, publish to crates.io, ASF distribution servers, and Git tags | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Backport Criteria | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| A release branch is a stabilization branch for an imminent or recent patch | ||||||||||||||||||||||||||||||||||
| release. The bar for landing a change on a release branch is therefore | ||||||||||||||||||||||||||||||||||
| _higher_ than the bar for landing on `main`, not lower. These criteria define | ||||||||||||||||||||||||||||||||||
| what is eligible for backport; the [Backport Workflow](#backport-workflow) | ||||||||||||||||||||||||||||||||||
| below describes the mechanics. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| DataFusion follows Cargo SemVer, with breaking changes allowed at major | ||||||||||||||||||||||||||||||||||
| version boundaries — see the [API health policy] for the full framing of | ||||||||||||||||||||||||||||||||||
| public Rust and SQL API stability. Patch releases (`x.y.z`, `z ≥ 1`) carry | ||||||||||||||||||||||||||||||||||
| fixes only and never introduce new features or breaking changes. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Eligible for backport | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| - **Security fixes.** Fixes for known or reported security issues should be | ||||||||||||||||||||||||||||||||||
| backported to every actively maintained release branch. | ||||||||||||||||||||||||||||||||||
| - **Correctness fixes.** Fixes for queries that produce incorrect results, | ||||||||||||||||||||||||||||||||||
| panics, data loss, or crashes. If the fix itself changes user-visible SQL | ||||||||||||||||||||||||||||||||||
| semantics to make a wrong result right, follow [Behavior changes] below. | ||||||||||||||||||||||||||||||||||
| - **Stability and regression fixes.** Fixes for regressions introduced in the | ||||||||||||||||||||||||||||||||||
| current release line, hangs, deadlocks, memory leaks, or other availability | ||||||||||||||||||||||||||||||||||
| issues. | ||||||||||||||||||||||||||||||||||
| - **Build, CI, and test fixes** required to keep the branch buildable and | ||||||||||||||||||||||||||||||||||
| releasable. | ||||||||||||||||||||||||||||||||||
| - **Documentation fixes** that correct documentation about behavior already | ||||||||||||||||||||||||||||||||||
| in the release. Documentation for behavior that exists only on `main` does | ||||||||||||||||||||||||||||||||||
| not belong on a release branch. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Not recommended for backport | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| - **New features**, including new SQL functions, new optimizer rules, new | ||||||||||||||||||||||||||||||||||
| configuration options, new public APIs, and new file-format support. Land | ||||||||||||||||||||||||||||||||||
| on `main` and ship in the next major release. | ||||||||||||||||||||||||||||||||||
| - **Breaking API changes** of any kind, Rust or SQL. DataFusion makes | ||||||||||||||||||||||||||||||||||
| breaking changes only at major version boundaries — see [API health policy]. | ||||||||||||||||||||||||||||||||||
| - **Refactors and cleanup** that do not fix a bug, even if they are correct. | ||||||||||||||||||||||||||||||||||
| - **Performance improvements** that are not also correctness or stability | ||||||||||||||||||||||||||||||||||
| fixes. Land on `main`. | ||||||||||||||||||||||||||||||||||
| - **Dependency upgrades**, except when the upgrade itself is the security or | ||||||||||||||||||||||||||||||||||
| correctness fix and there is no narrower alternative. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Behavior changes | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| A "behavior change" is any fix that alters user-visible results: SQL | ||||||||||||||||||||||||||||||||||
| semantics (values, ordering, types, null handling), error messages that | ||||||||||||||||||||||||||||||||||
| downstream users may rely on, plan output, or default configuration values. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Behavior-changing fixes need extra scrutiny on a release branch because | ||||||||||||||||||||||||||||||||||
| users upgrading between patch versions do not expect their queries to start | ||||||||||||||||||||||||||||||||||
| returning different results. Before opening the backport PR: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 1. State on the release tracking issue _why_ the change must ship in this | ||||||||||||||||||||||||||||||||||
| patch release rather than the next major. | ||||||||||||||||||||||||||||||||||
| 2. Describe the previous and new behavior with example queries and results. | ||||||||||||||||||||||||||||||||||
| 3. Wait for explicit acknowledgment from the release manager or another | ||||||||||||||||||||||||||||||||||
| committer on the tracking issue. | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+112
to
+118
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure we need to gate the creation of a backport PR on acknowledgement from the release manager -- making the backport PR is pretty simple, and I would expect that 2. (describing previous behavior, etc) would have already been done on the main issue Explaining why you want the change in the patch release is a good idea though. |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| If in doubt, default to "land on `main`, ship in the next major." | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Who decides | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The release manager for the active release line is the final reviewer of | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think practically any committer can commit to a release branch too -- so maybe we can make ti clear that others can merge PRs to the release branch, though the release manager will review them |
||||||||||||||||||||||||||||||||||
| what goes into the patch release. They coordinate via the release tracking | ||||||||||||||||||||||||||||||||||
| issue (for example, the [release issue for 50.3.0]). Anyone may propose a | ||||||||||||||||||||||||||||||||||
| backport by opening a backport PR and linking it from the tracking issue; | ||||||||||||||||||||||||||||||||||
| inclusion is the release manager's call. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ### Active release branches | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| DataFusion does not maintain Long-Term Support branches. In general only the | ||||||||||||||||||||||||||||||||||
| most recent `branch-NN` is actively maintained for backports. A branch is | ||||||||||||||||||||||||||||||||||
| considered closed once the next major release branch has been cut and the | ||||||||||||||||||||||||||||||||||
| release manager closes its tracking issue; users who need a fix beyond that | ||||||||||||||||||||||||||||||||||
| point should upgrade to the next major. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Security fixes are an exception: a maintainer may choose to backport a | ||||||||||||||||||||||||||||||||||
| critical security fix to an older branch even after it would otherwise be | ||||||||||||||||||||||||||||||||||
| closed. Discuss on the dev list or in a tracking issue before doing so. | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+140
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is true, but I also don't think I have heard opposition to doing more releases -- it is more of a bandwidth thing. Maybe we can soften the language a little like this:
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ## Backport Workflow | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The usual workflow is: | ||||||||||||||||||||||||||||||||||
|
|
@@ -121,3 +201,5 @@ This PR: | |||||||||||||||||||||||||||||||||
| [example backport pr]: https://github.com/apache/datafusion/pull/18131 | ||||||||||||||||||||||||||||||||||
| [additional backport pr example]: https://github.com/apache/datafusion/pull/20792 | ||||||||||||||||||||||||||||||||||
| [testing documentation]: testing.md | ||||||||||||||||||||||||||||||||||
| [api health policy]: api-health.md | ||||||||||||||||||||||||||||||||||
| [behavior changes]: #behavior-changes | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have made error message changes in minor releases before and it seems like that would be ok to me, to be honest.
The rest of this sounds good