Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
system.module
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
24 Apr 2019 at 19:50 UTC
Updated:
26 Sep 2023 at 21:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
fengtanComment #3
poker10 commentedThis function is still in Drupal 10.1.x-dev, see: https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/lib/Drupal/Core/Updater/Module.php#L84
It seems like it has no direct usages in D10 as well. We need to probably clarify the purpose of this function to decide.
Comment #4
smustgrave commentedSearching the repo I can't find any reference.
Searched http://grep.xnddx.ru/search?text=getSchemaUpdates&filename= and also didn't find any reference. Probably safe to remove as dead code.
Comment #5
nitin_lamaComment #6
nitin_lamaIf we need to remove the unused code, here's a patch to remove it. I couldn't find any reference in the core as well.
Comment #7
nitin_lamaComment #8
smustgrave commentedLets see what the committer says.
Comment #9
catchBefore removing something that's unused, we should check whether it's used in contrib (in this case no), but also why it's unused.
I ran
git log -S getSchemaUpdatesand the last mention wasI went back to the first mention:
And it shows the method being added but no usages.
As far as I can tell this was unused when it was added in 2009 and we've just been occasionally moving it around since.
However, it's a public method on a core class so I think we should deprecate it first.
Comment #11
elberI will work on it.
Comment #12
elberHi I deprecated the method and also created a change record, please
https://www.drupal.org/node/3359445
Comment #13
smustgrave commentedIf we are going to deprecate it think we should also add trigger_error for it.
Comment #15
elberComment #16
smustgrave commentedShould follow how other methods are deprecated
@trigger_error(__METHOD__ . '() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. There is no replacement. See https://www.drupal.org/node/3337942', E_USER_DEPRECATED);
Comment #17
andypostunpublished CR, it needs deprecation test additionally to #16 https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...
Comment #18
catchWe don't need a deprecation test unless there's an actual bc layer with logic in it or similar, when the deprecation is 'just' the phpdoc and trigger_error() we can skip testing for those.
Comment #21
quietone commentedComment #22
smustgrave commentedTrigger_error looks good, right version.
CR reads well and is with the right branches.
Think this is good to go.
Comment #23
catchI think this should just be deprecated with no replacement, it's not replaced by hook_update_N(), it's about finding out whether a module has updates that need to be run. No-one is likely to use this since it's very internal to core (except maybe drush but they won't be using this either).
Or we could point people to \Drupal::service('update.update_hook_registry')->getAvailableUpdates
Comment #24
quietone commentedI went with giving people an option, to use
\Drupal::service('update.update_hook_registry')->getAvailableUpdates. I also updated the CR.Comment #25
catchThanks that looks good, back to RTBC.
Comment #26
andypostUsing MR, RTBC++
Comment #28
longwaveCrediting @karolinam as requested in the IS.
Comment #29
longwaveCommitted 8e42cef and pushed to 11.x. Thanks!
Also published the change record.