Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
There's 2 @todo left in code
$ git grep 2080823
core/modules/field/field.purge.inc:91: // @todo Revisit after https://www.drupal.org/node/2080823.
core/modules/field/field.purge.inc:119: // @todo Revisit after https://www.drupal.org/node/2080823.
Steps to reproduce
Proposed resolution
Remove the @todo block of code
Remaining tasks
Review
Commit
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 2343517-20.patch | 1.62 KB | quietone |
| #13 | 2343517-10-reroll.patch | 1.57 KB | joelpittet |
| #10 | 2343517-10.patch | 1.58 KB | joelpittet |
| #4 | 2343517-4.patch | 2.3 KB | rteijeiro |
Comments
Comment #1
andypostComment #2
xjmComment #3
xjmComment #4
rteijeiro CreditAttribution: rteijeiro commentedNot sure if this is what it's expected. Let me know if not.
Comment #6
andypostComment #8
Mile23Moving to 8.2.x and Major because it's almost a duplicate of #2782009: Create a field with the same name as one being purged results in data destruction of the new field which is critical.
Comment #10
joelpittetNot sure if this is major, but here's a re-roll of #4 which only affects
core/modules/field/field.purge.inc
Comment #12
joelpittetInteresting failure:
Comment #13
joelpittetReroll
Comment #20
quietone CreditAttribution: quietone as a volunteer commentedcatch mentioned that this needed a reroll in #bugsmash.
So, here is the reroll. It is small so there is no interdiff.
Comment #21
dwwhttps://www.drupal.org/pift-ci-job/2222363 has some random fails. Requeued.
Is this really major?
I’m not sure why removing this code is so important. Can someone give this a real summary?
Thanks!
-Derek
Comment #22
longwaveAgree this is not major, downgrading - IMO this could even be either a minor bug or a normal task.
Comment #23
larowlanI agree, this is a task not a bug
Comment #24
quietone CreditAttribution: quietone as a volunteer commentedUpdated the IS. Anything else needed?
And starting tests again.
Comment #25
quietone CreditAttribution: quietone as a volunteer commentedDue to feedback in #bugsmash from darvanen I did some research. I didn't learn much. The code being removed is not tested by any test I could find. I did not find any supporting information in the related issues. Is it possible to have a field without an entity type?
It is only the issue summary that says to remove this.
@andypost, can you comment?
Comment #26
longwaveThis was originally added in #2081609: field purge should bail out on unknown entity types seemingly as protection against purging a field that was in some way related to a disabled module; previously I guess it would crash here if something that provided the entity type no longer existed as it was disabled. Now we no longer have the concept of disabled modules and you cannot uninstall a module without removing all of its content and fields first, I think this is safe to remove again.
Comment #27
dwwYeah, patch looks good. #26 sounds persuasive / reasonable. Happy to see this code and the comments go.
Thanks!
-Derek
Comment #29
SpokjeBack to RTBC per #26 after #3255836: Test fails due to Composer 2.2 solved the unrelated test failure.
Comment #31
catchCommitted/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!