Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
This has a @todo to remove once 8.6.x is no longer supported, which is very much the case with 9.1.x.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#21 | 3130341-5.patch | 3.62 KB | longwave |
Comments
Comment #2
jungleComment #3
longwaveThis now only calls the parent, so the entire method can be removed?
This was public static, it seems unlikely anyone would call it - but in theory they could. Do we have to go through the deprecation process?
One call in contrib suggests we might: http://grep.xnddx.ru/search?text=fixSerializedExtensionObjects&filename=
Comment #4
jungleThanks @longwave!
Re #3.2, It's an internal method, not sure if a BC layer is necessary.
Comment #5
jungleRemoving unused use statement.
Comment #6
catchhttp://grep.xnddx.ru/search?text=fixSerializedExtensionObjects&filename= is test-only, and explicitly references it's a workaround because the method hasn't been removed. That seems quite unique so I think we can go ahead and remove in a minor, but we should file an issue against lightning_media to be helpful.
Comment #7
jungleIssue filed to lightning_media [#3130552[ and CR created
Comment #8
longwave#5 looks good.
Comment #9
hchonovI would welcome such decision, however I have to ask whether this is documented officially? Do we already remove updates introduced in branches we don't support with the new version?
Comment #10
catchAll updates prior to Drupal 8.8 were removed in 9.0.x, change record is here: https://www.drupal.org/node/3098327 and links to various issues. Short version is that sites have to update to 8.8 or 8.9 prior to going to 9.0.
There is an open issue to discuss the cut-off point for Drupal 10 #2942096: [policy, no patch] Remove old update hooks prior to each major version (Drupal 10 and later).
At one point we were also discussing removing very old updates (< 8.6 era) from 8.9.x, but then realised it would be a lot more work than it saved after having done this for 9.0.x and run into various issues (like spotty hook_update_last_removed() support).
Comment #12
xjmThis was added for #3031128: Update from 8.6.7 to 8.6.8 warnings - Drupal\Core\Extension\Extension has no unserializer, which in turn was a fix for the update path for #2701829: Extension objects should not implement \Serializable, and both are indeed no longer part of the supported upgrade path. You have to update to 8.8.x in order to update to D9, so the functionality is no longer needed.
I confirmed that the only other non-test caller for this in 8.8.x was
update_fix_compatibility()
, which has also been killed with fire. And that use in Lightning Media 8.x-3.x (the supported branch for 8.8.x) is only in a test anyway. I pinged the Lightning maintainers a heads-up as well.Probably we should have deprecated this properly, but nonetheless, I think we should go ahead with this change in 9.1.x and 9.0.x as well.
Committed to 9.1.x and fixed up the CR a little. Leaving RTBC against 9.0.x to discuss the backport.
Comment #13
jungleRerolled a patch for 8.8.x, in case it's needed. The patch in #5 should apply to 8.9.x but does not apply to 8.8.x because of the difference of file path.
Comment #15
hchonovThe patch to be committed should be the last one on the issue. When providing re-rolls for earlier releases please also provide the patch for the target of the issue, which is now 9.0.x.
Comment #16
jungleRe #15, the patch in #5 was valid for 9.1.x and is still vaild for 9.0.x, that's why no 9.0.x patch uploaded explicitly.
Comment #17
longwave#5 is still OK for 9.0.x - it is dead code that does nothing now so I think we are OK to remove it there.
This cannot go back to 8.9.x or 8.8.x as update_fix_compatibility() still need it, as the test fails show - we can't remove any of this for BC reasons in D8.
Comment #19
jungle@longwave thank you!
Setting back to RTBC. Failure of 8.8.x patch is irrelevant.
Comment #20
xjmRight, the backport under discussion is 9.0.x only. Thanks!
Comment #21
longwaveReuploading #5 for clarity, the RTBC auto test is looking at #13 which means it will fail in 2 days when it gets retested again.
Comment #22
xjmIf we do backport this to 9.0.x, we'll want to do so before RC.
Comment #24
catchAgreed with backporting this, zero disruption and one less thing in our most-critical-bug-prone system.
Committed 7e93a68 and pushed to 9.0.x. Thanks!
Comment #25
xjm