findepi commented on code in PR #13701: URL: https://github.com/apache/datafusion/pull/13701#discussion_r1877926263
########## docs/source/library-user-guide/api-health.md: ########## @@ -19,11 +19,41 @@ # API health policy -To maintain API health, developers must track and properly deprecate outdated methods. +DataFusion is used extensively as a library and has a large public API, thus it +is important that the API is well maintained. In general, we try to minimize +breaking API changes, but they are sometimes necessary. + +When possible, rather than making breaking API changes, we prefer to deprecate +APIs to give users time to adjust to the changes. + +## Breaking Changes + +In general, a function is part of the public API if it appears on the [docs.rs page] + +Breaking public API changes are those that _require_ users to change their code +for it to compile, and are listed as "Major Changes" in the [SemVer +Compatibility Section of the cargo book]. Common examples of breaking changes: + +- Adding new required parameters to a function (`foo(a: i32, b: i32)` -> `foo(a: i32, b: i32, c: i32)`) +- Removing a `pub` function +- Changing the return type of a function + +When making breaking public API changes, please add the `api-change` label to +the PR so we can highlight the changes in the release notes. + +[docs.rs page]: https://docs.rs/datafusion/latest/datafusion/index.html +[semver compatibility section of the cargo book]: https://doc.rust-lang.org/cargo/reference/semver.html#change-categories + +## Deprecation Policy + When deprecating a method: -- clearly mark the API as deprecated and specify the exact DataFusion version in which it was deprecated. -- concisely describe the preferred API, if relevant +- Mark the API as deprecated using `#[deprecated]` and specify the exact DataFusion version in which it was deprecated +- Concisely describe the preferred API to help the user transition + +The deprecated version is the next version which will be released after the +deprecation PR is merged. For example, if the next scheduled release `41.0.0`, +and a method is deprecated in a PR, the deprecated version will be `41.0.0`. Review Comment: Let's write this in more actionable terms. ie. how exactly a contributor is supposed what to put in the `since = ...` string? maybe we can increase version to next as part of the release process and then we could say "the next version to be released can be found in top-level cargo.toml file of the project" ########## docs/source/library-user-guide/api-health.md: ########## @@ -19,11 +19,41 @@ # API health policy -To maintain API health, developers must track and properly deprecate outdated methods. +DataFusion is used extensively as a library and has a large public API, thus it +is important that the API is well maintained. In general, we try to minimize +breaking API changes, but they are sometimes necessary. + +When possible, rather than making breaking API changes, we prefer to deprecate +APIs to give users time to adjust to the changes. + +## Breaking Changes + +In general, a function is part of the public API if it appears on the [docs.rs page] + +Breaking public API changes are those that _require_ users to change their code +for it to compile, and are listed as "Major Changes" in the [SemVer +Compatibility Section of the cargo book]. Common examples of breaking changes: + +- Adding new required parameters to a function (`foo(a: i32, b: i32)` -> `foo(a: i32, b: i32, c: i32)`) +- Removing a `pub` function +- Changing the return type of a function + +When making breaking public API changes, please add the `api-change` label to +the PR so we can highlight the changes in the release notes. + +[docs.rs page]: https://docs.rs/datafusion/latest/datafusion/index.html +[semver compatibility section of the cargo book]: https://doc.rust-lang.org/cargo/reference/semver.html#change-categories + +## Deprecation Policy + When deprecating a method: -- clearly mark the API as deprecated and specify the exact DataFusion version in which it was deprecated. -- concisely describe the preferred API, if relevant +- Mark the API as deprecated using `#[deprecated]` and specify the exact DataFusion version in which it was deprecated Review Comment: Make it more copy-paste friendly ``` #[deprecated(since = "...", note = "...")] ``` this may be important given that there is no static check which would flag `#[deprecated]` without `since` attribute and those sometimes make to the main branch (https://github.com/apache/arrow-rs/pull/6782) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
