doctrine-website
doctrine-website copied to clipboard
doctrine
Make addressing deprecations acceptable in a patch release
Prompted by https://github.com/doctrine/orm/pull/10736
Addressing a deprecation is not a bugfix, it does not make the software more stable. Deprecations are annoying though, and unless we switch to using the ~ operator instead of the ^ operator for our dependencies, we might get new ones out of the blue. I propose to explicitly allow PRs that address deprecations in patch releases, so that users do not have to wait until the next minor to have fewer deprecations.
Hm, a tricky one.
Maybe the question we should ask yourselves first is: "Do we find it acceptable to raise our own minimum requirements in bugfix releases?"
Assume we depend on version ^1.0 of some package. Now this package releases 1.1.0 (or, beware! 1.0.1?), adding a deprecation and telling us to switch to a new API that was part of that release.
In that case, I think we should not raise minimum requirements in our own next bugfix release: It would risk to cut off our users from this and following bugfixes in case they cannot (for whatever reason) accept the new (stricter) requirements.
But, do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package, just to remedy the deprecation? IMHO the answer is "no" as well.
So, the only way I see to deal with such deprecations in a graceful way is to wait for our own next minor release, where we can raise minimum deps to a version where the new/alternative API is availabe, and then use that.
The consequences of this is that we should either release minors more often, potentially as often as one of our dependencies has a new minor version, and/or we should prevent the new minor version from being installed by using the ~ operator rather than the ^ operator. Not great.
I think the best course of action might be to tolerate the feature/version detection code in patch releases.
Isn’t the point of having packages and releases also to de-couple regarding time, that is, everyone dealing with upgrades at a time they see fit?
What pressure would it put on us having to release a minor every time a depended-upon package adds a deprecation?
Also, as a library we should try (IMHO) to be as liberal as possible (and practical) regarding dependency constraints… so, not use ~ and not raise minimum requirements without or little reason (I know that’s opinionated 🔥).
Maybe @nicolas-grekas can tell us how the Symfony project addresses this concern?
Maybe you can say that a deprecation is an announcement that something needs to be done in the future. We will address it, but at a time that fits our release cycle.
It’s not a bug that needs more immediate attention.
What pressure would it put on us having to release a minor every time a depended-upon package adds a deprecation?
Releasing a minor version of a Doctrine package is not such big deal now that we are using laminas/automatic-releases. Maybe this is the way. The last minor version of doctrine/orm was quite big so maybe it would be a good thing in that it would force us to release smaller minors more often.
There's a drawback to that though: if the minor contains new deprecations, then our users will be trading indirect deprecations for other deprecations (from doctrine/orm directly or through some the framework integration package).
So maybe the real question we should ask ourselves is: why do users want deprecations to be fixed? That's probably because it clutters the output of phpunit, or the Symfony inspector. Maybe indirect deprecations shouldn't be shown by default?
That's the big Yes from me because it enables smooth transitions, by allowing decoupling of dependency updates. It allows escaping from dependency hell. 👍
So should we allow addressing a deprecation in a patch release iff it can be done without bumping the constraints, or introducing feature detection / version detection (because that's too complex)? Is that what you both are saying? Or should it be more simple than that and should https://github.com/doctrine/orm/pull/10737 also have been retargeted in your opinion?
@nicolas-grekas thank you for your swift response!
That's the big Yes from me because it enables smooth transitions,
I'm afraid I don't exactly get what you're referring to... 🤔 Could you please try to re-phrase that?
Do you mean not tightening requirements in bugfix releases, not using the ~ operator and/or not trying to address deprecations too eagerly?
As bug fix, so that PR did right being merged on 2.15. This is how we can enable the community to adopt new versions as quickly as possible, by making it as easy as possible to run deprecation-free (thus accelerating the adoption of the next major)
I mean that depreciation should be addressed as bug fixes if that wasn't explicitly enough :)
And if that means having to bump requirements on bugfix releases, that’s ok for you?
I'm upvoting the clarification, but after the discussion with @mpdude, I'm no longer sure I agree with it. Addressing a deprecation is not a bugfix, there IMO no bug, just an inconvenience for developers. Maybe what should be challenged is the inconvenience? I mean what happens if addressing the deprecation causes an actual bug? @mpdude was that what you were worried about when saying "do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package"?
Anyway, we clearly need more Doctrine maintainers to review this, it seems to be quite controversial.
was that what you were worried about when saying "do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package"?
(*) Yes, kind of. In bugfixes, we want as little risk as possible. So, we should not add lots of detection/case switching logic there just to be able to avoid deprecations while keeping minimum requirements unchanged.
(*) If we want to address deprecations without such logic, we can probably only do it by raising minimum requirements.
If we raise requirements in bugfixes, people may not be able to upgrade to the new bugfix version although they were able to use the preceding bugfix.
In my opinion, a minor release is a promise that users get a certain feature set and that we will provide bugfixes to those users for a certain time. Raising minimum requirements means potentially excluding/dropping some of these users along the way, during the bugfix phase of said release.
Disclaimer:
What a package depends upon is an implementation detail, not part of its exposed API. So dependency/minimum requirement changes are irrelevant with regard to the version number that needs to be attached. Technically, raising requirements in a bugfix is not wrong.
It’s more of a practical question if we want to impose this additional constraint. “No” means we can address deprecations faster. “Yes” means we can guarantee more predictable upgrade paths/support periods.
Everything is a tradeoff.
Update/edit: Paragraphs marked with (*) have been challenged by Nicolas below.
There's absolutely no needs to bump any requirements, what a strange idea. I've spent years on Symfony following this process and I can tell by experience.
Not requiring a dep bump is precisely the reason why this should be fixed in patch releases. The other direction, fixing depreciation as a feature, means correlating the ability to run deprecation-free to a version bump of another dep, typically PHP. And THAT creates dependency hell, which is defined as the extremely high cost to upgrade dependencies because of chains of dependency requirements. Like the one we're talking about.
Feels like we have many evils to choose from, and we should determine what's the lesser evil.
- potential unstability due to complex detection code
- cluttered deprecation reports for users + deprecation reported as Github issues for maintainers + trading deprecations fixes for new deprecations coming with a new minor
- tightened dependencies
I think I agree with Nicolas, and that we should be super careful when reviewing code that addresses deprecation, but let it land on a patch release. We should insist on having the most simple fix on a patch release, even if that means postponing the real fix to the next minor.
Well if a depended-upon package says “the API you’re using has been deprecated during our recent 1.1.0 release, use the new API instead”… what can we do?
If we switch to that new API, we need to bump to ^1.1, don’t we?
Edit: I mean bumping our own Composer requirements, not our version number.
My experience is that the risk we're talking about is minimal. It's usually just a class_exists() check or similar. No need to FUD ourselves :)
Ah so you’re saying not bump our own composer requirements, but add logic to recognize the new API and avoid the deprecation, and do that in a bugfix release?
👌🏻. But that also means you avoid or forbid bumping requirements in bugfixes, do you?
I can endorse the current state ✌🏻
In addition, depending on whether and how Nicolas answers my last question, do we want to state either
- that no requirement bumps in bugfixes are allowed (not limit that to the context of deprecations?)
- minor releases may raise requirements if that in turn helps simplify our own codebase (ie get rid of
class_existschecks, to stick with the example) and/or use new APIs, but we don’t raise them just for the sake of it?
👌🏻. But that also means you avoid or forbid bumping requirements in bugfixes, do you?
correct
of course, exception may happen, but the rule of thumb is that bumping a dep in a bugfix should not force a dep bump of a transitive dep (php especially of course, but others could be checked)
Just realized that since we have no fixed/guaranteed release cycle like Symfony, in fact no promise is given.
We can release a new minor at any time, and once we do, we stop bugfixing the previous minor, right?
So, for someone using for example 2.15.1 and being constrained by requirements, the journey may end at any time. It does not matter if we bump reqs and name the result 2.15.2 or 2.16.0, the net effect is the same.
Another rule of thumb: don't bump to the next minor unless all deprecations of the current one are fixed (which is clearly not the case today)
The corollary is: make the CI fail when deprecations are raised, and don't release when the CI is not green ;)
We can release a new minor at any time, and once we do, we stop bugfixing the previous minor, right?
That's the case.
It seems like the ~ vs. ^ conversation moved into the background. While one topic was not bumping 1.0 to 1.1 in the dependencies under certain conditions, newer releases of the same dependency would introduce new deprecations that couldn't be addressed when e.g. ^1.1 is used and a new 1.3.0 of a dependency got released.
Any kind of restriction contributes to dependency hell, so no, please don't put an upper limit to deps with ~. Deprecations are meant to flow to en users and using ~ would be a way to actively defeat the communication intentions of the author of deps.
It's fine if a non maintained release start triggering deprecation because trasitive deps moved forward. There are many tools to deal with deprecations any way (and upgrading is one of them)