Skip to content

Commit 8df75c3

Browse files
authored
Document guidance on how to evaluate breaking API changes (#20584)
## Which issue does this PR close? ## Rationale for this change DataFusion does make API changes from time to time, and that is a normal part of software development. However, it is important to evaluate the impact of those API changes on downstream users and to ensure that the benefits of the change are clear to those users. I found a few times where API changes were made with the justification that "some APIs in DataFusion are cleaner" or "this is more consistent with other APIs". While those may be valid justifications, it is painful for downstream users who have change their code to accommodate the API change when they get nothing in return This most recently happened in this PR - #19790 (review) thus I think the contributor guide should include some guidance on how to evaluate breaking API changes and to ensure that the benefits of the change are clear to downstream users. ## What changes are included in this PR? Polish up the API guidance section ## Are these changes tested? By CI ## Are there any user-facing changes? Better / clearer docs
1 parent 3a23bb2 commit 8df75c3

1 file changed

Lines changed: 50 additions & 21 deletions

File tree

docs/source/contributor-guide/api-health.md

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,48 +19,77 @@
1919

2020
# API health policy
2121

22-
DataFusion is used extensively as a library and has a large public API, thus it
23-
is important that the API is well maintained. In general, we try to minimize
24-
breaking API changes, but they are sometimes necessary.
22+
DataFusion is used extensively as a library in other applications and has a
23+
large public API. We try to keep the API well maintained and minimize breaking
24+
changes to avoid issues for downstream users.
2525

26-
When possible, rather than making breaking API changes, we prefer to deprecate
27-
APIs to give users time to adjust to the changes.
26+
## Breaking API Changes
2827

29-
## Upgrade Guides
30-
31-
When making changes that require DataFusion users to make changes to their code
32-
as part of an upgrade please consider adding documentation to the version
33-
specific [Upgrade Guide]
34-
35-
[upgrade guide]: ../library-user-guide/upgrading/index
28+
### What is the public API and what is a breaking API change?
3629

37-
## Breaking Changes
38-
39-
In general, a function is part of the public API if it appears on the [docs.rs page]
30+
In general, an item is part of the public API if it appears on the [docs.rs page].
4031

4132
Breaking public API changes are those that _require_ users to change their code
4233
for it to compile and execute, and are listed as "Major Changes" in the [SemVer
43-
Compatibility Section of the cargo book]. Common examples of breaking changes:
34+
Compatibility Section of the Cargo Book]. Common examples of breaking changes include:
4435

4536
- Adding new required parameters to a function (`foo(a: i32, b: i32)` -> `foo(a: i32, b: i32, c: i32)`)
4637
- Removing a `pub` function
4738
- Changing the return type of a function
39+
- Adding a new function to a `trait` without a default implementation
40+
41+
Examples of non-breaking changes include:
42+
43+
- Marking a function as deprecated (`#[deprecated]`)
44+
- Adding a new function to a `trait` with a default implementation
45+
46+
### When to make breaking API changes?
47+
48+
When possible, we prefer to avoid making breaking API changes. One common way to
49+
avoid such changes is to deprecate the old API, as described in the [Deprecation
50+
Guidelines](#deprecation-guidelines) section below.
51+
52+
If you do want to propose a breaking API change, we must weigh the benefits of the
53+
change with the cost (impact on downstream users). It is often frustrating for
54+
downstream users to change their applications, and it is even more so if they
55+
do not gain improved capabilities.
56+
57+
Examples of good reasons for making a breaking API change include:
4858

49-
When making breaking public API changes, please add the `api-change` label to
50-
the PR so we can highlight the changes in the release notes.
59+
- The change allows new use cases that were not possible before
60+
- The change significantly enables improved performance
61+
62+
Examples of potentially weak reasons for making breaking API changes include:
63+
64+
- The change is an internal refactor to make DataFusion more consistent
65+
- The change is to remove an API that is not widely used but has not been marked as deprecated
66+
67+
### What to do when making breaking API changes?
68+
69+
When making breaking public API changes, please:
70+
71+
1. Add the `api-change` label to the PR so we can highlight the changes in the release notes.
72+
2. Consider adding documentation to the version-specific [Upgrade Guide] if the required changes are non-trivial.
5173

5274
[docs.rs page]: https://docs.rs/datafusion/latest/datafusion/index.html
5375
[semver compatibility section of the cargo book]: https://doc.rust-lang.org/cargo/reference/semver.html#change-categories
5476

77+
## Upgrade Guides
78+
79+
When a change requires DataFusion users to modify their code as part of an
80+
upgrade, please consider documenting it in the version-specific [Upgrade Guide].
81+
82+
[upgrade guide]: ../library-user-guide/upgrading/index.rst
83+
5584
## Deprecation Guidelines
5685

5786
When deprecating a method:
5887

5988
- Mark the API as deprecated using `#[deprecated]` and specify the exact DataFusion version in which it was deprecated
6089
- Concisely describe the preferred API to help the user transition
6190

62-
The deprecated version is the next version which contains the deprecation. For
63-
example, if the current version listed in [`Cargo.toml`] is `43.0.0` then the next
91+
The deprecated version is the next version that introduces the deprecation. For
92+
example, if the current version listed in [`Cargo.toml`] is `43.0.0`, then the next
6493
version will be `44.0.0`.
6594

6695
[`cargo.toml`]: https://github.com/apache/datafusion/blob/main/Cargo.toml
@@ -76,4 +105,4 @@ pub fn api_to_deprecated(a: usize, b: usize) {}
76105

77106
Deprecated methods will remain in the codebase for a period of 6 major versions or 6 months, whichever is longer, to provide users ample time to transition away from them.
78107

79-
Please refer to [DataFusion releases](https://crates.io/crates/datafusion/versions) to plan ahead API migration
108+
Please refer to [DataFusion releases](https://crates.io/crates/datafusion/versions) to plan API migration ahead of time.

0 commit comments

Comments
 (0)