SAVED_DELETED is not used in D11, D10, D9, or D8 and should be removed

Created on 27 May 2022, over 2 years ago
Updated 10 January 2025, 25 days ago

Problem/Motivation

The constant SAVED_DELETED, defined in core/includes/common.inc is not used anywhere in core since Drupal 7. It is not used in Drupal 8 or Drupal 9 or Drupal 10.

Absent a use case for this constant, it should be removed from core.

πŸ“Œ Task
Status

Needs review

Version

11.0 πŸ”₯

Component

base system

Created by

πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡Έ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.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Think something like that.

  • Pipeline finished with Success
    24 days ago
    Total: 320s
    #393204
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Unfortunately I can't mark it now.

  • πŸ‡ΊπŸ‡Έ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.

    • catch β†’ committed f5d1ca72 on 11.x
      Issue #3282744 by tr, smustgrave, ghost of drupal past, geek-merlin,...
  • πŸ‡¬πŸ‡§United Kingdom catch

    Committed/pushed to 11.x, thanks!

  • πŸ‡ΊπŸ‡Έ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.

Production build 0.71.5 2024