Review backward compatibility policy

[nicoddemus]

Not changed much, only updated with new best practices and described how we will remove functionality in two steps instead of removing them in major releases directly.

[nicoddemus]

and keep it around for at least two minor releases

@RonnyPfannschmidt because of this requirement, I think we should after 4.0 use a different scheme than we are using now with RemovedInPytest4Warning.

When we deprecate something we don't know exactly when the next major release will be. This makes it hard to track which of the functionalities marked with RemovedInPytest4Warning can be removed when pytest 4.0 comes around. For example, suppose we release 3.9 and then 4.0. We need to track mentally what has been deprecated in 3.9, because they can't really be removed in 4.0. Worse, they will carry the RemovedInPytest4Warning name with them until the 5.0 release, because we explicitly can only remove things in major releases. Because this, we can't deprecate anything else from now on, because this will lock us into getting 4.0 out if we want to avoid slipping RemovedInPytest4Warning into the 4.X series until 5.0.

To avoid this problem, here's what I propose:

We add a version parameter to PytestDeprecationWarning. When we deprecate something, we always know what the next minor release will be (for example at the moment we know the next release is 3.9). When a new major release happens, we just need to go over the deprecation warning instances and see which have been around for more than 2 minor releases, those are the ones we should turn into errors and remove later.

The more we refactor the internal code and improve the API, the more I see getting a very tight and consistent deprecation process is important to avoid disrupting our users.

Please let me know your thoughts. 😁

[obestwalter]

To avoid this problem, here's what I propose:

We add a version parameter to PytestDeprecationWarning. When we deprecate something, we always know what the next minor release will be (for example at the moment we know the next release is 3.9). When a new major release happens, we just need to go over the deprecation warning instances and see which have been around for more than 2 minor releases, those are the ones we should turn into errors and remove later.

Very good - I think that would make it much clearer.

One suggestion: how about naming the parameter in a clearer way, that even the casual source code reader knows what it is about. Something like deprecated_since[_version]?

If the docstring for the deprecation would also contain the "rules" then, this would be clear as day - e.g.

class PytestDeprecationWarning(PytestWarning, DeprecationWarning):
    """
    [Same stuff as before]

    Removals only happen in major releases, after a deprecation warning has been emitted for at least two minor versions.
    """

Don't know if this is the best way to phrase it, but I hope you get my drift.

[nicoddemus]

All around good suggestions. 👍

[coveralls]

Coverage increased (+0.02%) to 93.835% when pulling a4dd6ee on nicoddemus:update-backward-policy into 49800ea on pytest-dev:master.

[obestwalter]

Much more informative like that, even with a touch of philosophy about how to deal with warnings.

Just a question about --disable-pytest-warnings is this planned to be phased out then and be replaced with the standard means? I just ran a test with newest pytest using that flag and did at least not get a deprecation warning for it. If we don't need this anymore all the better - the amount of command line options for pytest is staggeringly vast, so a bit of pruning would not hurt :)

[nicoddemus]

I remove the mention about --disable-pytest-warnings because when this was written, it only suppressed pytest's own internal warnings. The standard warnings plugin was even introduced yet at the time.

Now --disable-pytest-warnings is an alias to --disable-warnings, which disables the entire warning summary (internal and external warnings). I thought I would remove it from here as it no longer is deprecation-specific, and mentioning the warning filters seems more productive as it is a more flexible and generic mechanism.

[obestwalter]

Makes perfect sense. So this alias could then be deprecated, I guess :)

[obestwalter]

To avoid this problem, here's what I propose:

We add a version parameter to PytestDeprecationWarning. When we deprecate something, we always know what the next minor release will be (for example at the moment we know the next release is 3.9). When a new major release happens, we just need to go over the deprecation warning instances and see which have been around for more than 2 minor releases, those are the ones we should turn into errors and remove later.

Very good - I think that would make it much clearer.

One suggestion: how about naming the parameter in a clearer way, that even the casual source code reader knows what it is about. Something like deprecated_since[_version]?

If the docstring for the deprecation would also contain the "rules" then, this would be clear as day - e.g.

class PytestDeprecationWarning(PytestWarning, DeprecationWarning):
    """
    [Same stuff as before]

    Removals only happen in major releases, after a deprecation warning has been emitted for at least two minor versions.
    """

Don't know if this is the best way to phrase it, but I hope you get my drift.

[RonnyPfannschmidt]

it makes me wonder if we should just switch to date based versions altogether

[nicoddemus]

it makes me wonder if we should just switch to date based versions altogether

Not sure, but I think this is a discussion for another time. 😁

[nicoddemus]

Anything else here @RonnyPfannschmidt, or can we merge this?