Skip to content
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

Add deprecation policy #1857

Closed
wants to merge 5 commits into from
Closed

Add deprecation policy #1857

wants to merge 5 commits into from

Conversation

Kludex
Copy link
Sponsor Member

@Kludex Kludex commented Sep 11, 2022

@tomchristie Should we present the deprecation policy as this PR proposes, or should we also include the versioning policy we aim to follow, and maybe discuss the branch strategy?

I'm mentioning the branch strategy in case we want to backport security patches to previous major versions.

In any case, what do you think about this PR?

@tomchristie
Copy link
Member

Thanks @Kludex. I think my main suggestion here would be to highlight that we use SEMVER, and then to clarify what that means in the context of python versions, pre-1.0, and deprecation warnings.

The bits which I think are obviously what we want here are then...

  • We use SEMVER. https://semver.org
  • With some further additional constraints...
  • While we are pre-1.0, we treat any SEMVER changes requiring a "major" bump as a change requiring a "minor" bump. We treat any SEMVER changes requiring a "minor" bump as a SEMVER change requiring a "patch" bump.
  • We'll support Python versions until they are EOL. Dropping a Python version is considered a backward incompatible change, and so requires a major version bump. (Or a "minor" version bump while we're still pre-1.0)

Less obvious to me is exactly what we would like wrt. warnings. Possibly the least confusing wrt. to SEMVER, would be...

  • Where possible we will warn of any upcoming backwards incompatible changes. Introducing a new warning requires a "minor" version bump. (Or a "patch" version bump while we're still pre-1.0).

That last one is not quite what we've talked about in the past, but actually I think it'd align more simply?

@Kludex
Copy link
Sponsor Member Author

Kludex commented Sep 18, 2022

We'll support Python versions until they are EOL. Dropping a Python version is considered a backward incompatible change, and so requires a major version bump. (Or a "minor" version bump while we're still pre-1.0)

I don't think this statement is a consensus on the Python community. Packages like coverage bump minor when dropping Python versions: https://coverage.readthedocs.io/en/6.4.1/changes.html#version-6-3-2022-01-25. I'd also prefer to bump minor here.

That last one is not quite what we've talked about in the past, but actually I think it'd align more simply?

Sure.

Also, I've been thinking... Maybe we can agree to not remove code that is deprecated before we are ready to 1.0 i.e. when we are about to release 1.0 we remove all code with warnings. What do you think? 🤔

@Kludex
Copy link
Sponsor Member Author

Kludex commented Sep 24, 2022

I've applied comments, and applied what the policy states on all warning statements.

Comment on lines +38 to +40
The format of the message should follow:

> *`code` is deprecated and will be removed in version `version`.*
Copy link
Member

Choose a reason for hiding this comment

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

From meeting: let's add info about when we add warnings.


### Feature Deprecation

Starlette will deprecate a feature in the next minor release after the feature is marked as deprecated.
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

This is the "when". Should the wording be changed? @adriangb

Copy link
Member

Choose a reason for hiding this comment

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

I think this is saying when we will deprecate the feature, the question was when do we put in deprecation warnings. Can users expect deprecation warnings to show up at any time, only in minor releases or only in major releases?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

The act of deprecating a feature is adding a deprecation warning. At least, this is how I understand.

Copy link
Member

@adriangb adriangb Oct 3, 2022

Choose a reason for hiding this comment

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

How about:

Suggested change
Starlette will deprecate a feature in the next minor release after the feature is marked as deprecated.
Starlette will deprecate (completely remove or break) a feature in the next major release after the feature is marked as deprecated (via a deprecation warning). Features can be marked as deprecated in any minor release.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I'll apply something on that line.

@Kludex
Copy link
Sponsor Member Author

Kludex commented Oct 3, 2022

@Kludex Kludex closed this Oct 3, 2022
@Kludex Kludex deleted the deprecation-policy branch October 3, 2022 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants