- πΊπΈUnited States tr Cascadia
Not exactly. We have to deprecate it (by a CR) and to remove it in D11.
I don't understand why this was moved back to NW.
The draft CR deprecating the constant was created on 21 Dec 2022 by @danielveza, as requested.
He then moved the issue to NR.So what needs to be done here to get this constant removed? It is not used by core or contrib in D11, D10, D9, or D8.
- πΊπΈUnited States smustgrave
Believe number #12 still stands. We would have to officially deprecate it in 11.2 and remove in 12 to be safe.
- πΊπΈUnited States tr Cascadia
What does that mean? The CR exists, are you saying it just needs to be published? Then this issue should not be NW because it is ready for a maintainer to publish. If the CR is not sufficient then why? If the CR needs to be changed, what's wrong with it?
- πΊπΈUnited States smustgrave
It actually has to be deprecated in code. Contrib or custom modules may be using orb
- πΊπΈUnited States tr Cascadia
#12 says deprecate by CR. That's why the status was NW.
There is no mention of deprecating in code.
That's why I'm asking. - Merge request !10878Issue #3282744 by tr: SAVED_DELETED is not used in D11, D10, D9, or D8 and should be removed β (Closed) created by smustgrave
- πΊπΈUnited States smustgrave
smustgrave β changed the visibility of the branch 3282744-saveddeleted-is-not to hidden.
- πΊπΈUnited States nicxvan
Yep that is how to deprecate constants now. I don't think this was defined when the issue was first created.
This is good cleanup!
I updated the CR to be more concise.
The history isn't needed in the CR it's on the issue.
I think I can mark it rtbc since I only changed the CR.
- π¬π§United Kingdom catch
@TR documentation on how to deprecate constants is here: https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... β
MR looks good to me.
- πΊπΈUnited States tr Cascadia
"@TR documentation on how to deprecate constants is here"
Yeah but that was only created in May 2024.
Back in May 2022 when this issue was opened there was NO mechanism for deprecating constants.What I'm hearing is that there is no good reason why this was put into NW two years ago, but now the solution has to be different because core changed. It shouldn't be this hard to get little things fixed.
- π¬π§United Kingdom catch
@TR, no when #12 was written, while there was no way to deprecate constants, we would remove constants that were unused by contrib in minor releases (probably), or if not, in a major release - in both cases with a change record since that was the only way to communicate the change.
It looks like @danielveza created the change record the same day, but forgot to come back and update this issue. Then also no-one else noticed or updated this issue in the intervening two years.
Had the issue been re-RTBCed, it could have been committed to either a major or minor release at that point.
During that time, we added better support for deprecating constants (not trivial because it can't be done at runtime, so it had to be added to phpstan), so that we don't break contrib modules relying on them, like the ones that Ghost of Drupal Past found in #9, and this can happen in any minor release now, no need to grep contrib etc.
I've now spent five minutes writing this up which I could have spent committing this issue if #25 hadn't been inaccurate.
- πΊπΈUnited States tr Cascadia
so that we don't break contrib modules relying on them, like the ones that Ghost of Drupal Past found in #9, and this can happen in any minor release now, no need to grep contrib etc.
@ghost was citing the russian site, which was never up-to-date. But I did my search on gitlib, which *is* up-to-date, and I did that search both before I opened the issue and also gave the link in #6. No active, published contrib was/is using this constant.
Yes, deprecating is a much better way, but that method didn't exist two years ago so I pro-actively searched contrib and presented my results out of an abundance of caution.
I've now spent five minutes writing this up which I could have spent committing this issue if #25 hadn't been inaccurate.
And if the desired fix back then was to change the CR "introduced in" version from 10.1 to 11.0, then that could have been done in 10 seconds two years ago, rather than throwing it into "needs work" with an inaccurate reason why. And then NO-ONE would have had to spend 5 or 10 or 30 minutes more dealing with this.