Add deprecation / API removal policy #6852
Conversation
alamb
added
parquet
|
I can't believe I'm saying this, but maybe 2 major releases would be more prudent 🤷 |
|
Thanks @etseidl -- I plan to leave this PR open for a while after publicizing it broadly, to allow maximal chance for comment.
I also vacillated between 1 release and 2 releases (and almost proposed 2 releases). I updated the proposal to be 2 releases |
|
I think a minimum of 1 major release, and then depending on the severity of the change (as in what is the alternative, what do behavior or changes to defaults fall into?) a judgement call is made. I think mandating 2 releases no matter what would disregard that there are various degrees of changes. |
README.md
Outdated
| #[deprecated(since = "51.0.0", note = "Use `date_part` instead")] | ||
| ``` | ||
|
|
||
| Deprecated APIs will be kept for at least two major releases after they were |
There was a problem hiding this comment.
The PR initially started with 1, now it's 2. This is not unimportant. The less the better from code maint. perspective
There was a problem hiding this comment.
Yes, I agree it is an important distinction. Let's see if any consensus emerges
|
My recommendation would be 2 major releases, but @brancz makes a very good point that it should be a recommendation and not necessarily a hard rule. |
|
Given the lag with which people upgrade to the latest major versions, we are about to cut version 54.0.0 and only 1/3 of the download base (according to crates.io)has upgraded to 53.0.0, 2 major versions is probably the right cadence. I do wonder if we might instead use a time bound, e.g. 6 months. This would not only be less restrictive, but also translate should be look to reduce the cadence of breaking releases in future as well. |
Rather than trying to come up with a process now, perhaps we can just make the policy more general and say "at the discretion of the committers" or something, and we can add a more formal policy if there are specific issues caused by the informal one |
|
I tried to incorporate all the feed back so far:
|
README.md
Outdated
| downstream Rust programs to still compile, but generate compiler warnings. This | ||
| gives downstream crates time to migrate prior to API removal. | ||
|
|
||
| All deprecated APIs are marked using the `#[deprecated]` attribute with both the |
There was a problem hiding this comment.
#[deprecated(since="...", note="...")] to decrease chances of creating new deprecations with missing since attr? (see #6782)
| first version they were deprecated in, and what new API to use instead. | ||
|
|
||
| ```rust | ||
| #[deprecated(since = "51.0.0", note = "Use `date_part` instead")] |
There was a problem hiding this comment.
like in DF deprecation guidelines, it it obvious how a contributor should fill the since field? eg is this the version string from cargo.toml, or next major from that?
There was a problem hiding this comment.
like in DF deprecation guidelines,
There was a problem hiding this comment.
I tried to do it in 9fcc60c
|
I think this PR offers enough guidance to be actionable and help improve predictability without committing us to a hard and fast rule. Thus I am going to merge it in and we can iterate going forward |
* Add deprecation / API removal policy * Increase proposal to 2 releases * change from policy to guidelines, add flexibility * prettier * Make instructions more actionable
Which issue does this PR close?
Closes #6851
Rationale for this change
As this crate matures, more predictability is good
What changes are included in this PR?
Add a proposed deprecation policy to the README
Options considered:
I am currently proposing 2 major releases, but would love to hear more input
Are there any user-facing changes?
Just docs, but new policy