Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

APE 2: Clarify next major release meaning in deprecation policy #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pllim
Copy link
Member

@pllim pllim commented Aug 27, 2024

Because I was very confused for a few hours. Thanks to @bsipocz for the clarification!

cc @astrofrog and @saimn as co-authors of the last revision.

in deprecation policy.

Co-authored-by: Brigitta Sipőcz <bsipocz@gmail.com>
@pllim pllim requested a review from eteq August 27, 2024 20:04
APE2.rst Outdated
@@ -137,7 +137,8 @@ The guidelines in relation to API changes and backward compatibility are as foll
* New deprecations should not be introduced in bugfix releases.
* If developers wish to make an API change at a point in time where the next
release is a major release, they should introduce the deprecation in the major
release and carry out the change in the following major release.
release and carry out the change in the following major release
(i.e., the second feature release after deprecation).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added remark clearly contradicts the second example in Release versioning and scheduling

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed; I think using "feature release" here alongside "minor release" may just add to the confusion, unless one wants to make the point that a major release is also a feature release (besides being an API-changing release and a bugfix release). The key points to me really are, as stated above, "the change itself must only be made in a major release" and "the deprecation warning shall be visible for at least a year before that".
But since the release cycle clearly allows for more than two minor/feature releases within a year, giving any number here misses the point imo.
Might be clearer to rephrase that entire bullet to "introduce the deprecation in the next feature release (meaning, regardless whether that happens to be a minor or major release) and carry out the deprecation in the following major release".
With the two conditions above, that might mean both that the time can be longer than one year, and that there is no other major release during the deprecation period (e.g. in that second example, deprecation added in 6.1.0, removal executed in 7.0.0 – 15 months later).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is "following major release" that confuses me.

If deprecation is added in 6.1.0, and then the deprecated code removed in 7.0, that is only about 6 months, according to https://github.com/astropy/astropy/wiki/Release-Calendar .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that only holds in the original wording (the case that the deprecation is also introduced in a major release): since major releases are at least twelve months apart, that would set a minimum time of one year.
In the rephrasing I suggested above, that would have to be "the major release following at least a year after the deprecation".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I reworded it. Better?

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems very straightforward, and is a good clarification.

A key question is whether we consider this a "revision" and want to spend the time to update Zenodo though... if so it should get an update to "date-last-revised:"...

@pllim
Copy link
Member Author

pllim commented Dec 10, 2024

whether we consider this a "revision"

Since I didn't really change the original meaning, maybe we can get away by not considering this a revision?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants