Follow-up for #3376376-26: HelpSectionManager::clearCachedDefinitions breaks the update system
We should be able to remove both places in 11.x - once the update is removed, we can also guarantee that the module has been updated to at least this schema version. Let's open a follow-up for that explicitly since there's no @deprecated tag or anything and probably a @todo linking to the issue as well.
$ git grep 10200 -- core/modules/help
core/modules/help/help.install:56:function help_update_10200(&$sandbox = NULL) {
core/modules/help/help.module:151: if (\Drupal::service('update.update_hook_registry')->getInstalledVersion('help') >= 10200) {
core/modules/help/src/HelpSectionManager.php:61: if ($this->searchManager && $version >= 10200) {
core/modules/help/tests/src/Functional/Update/HelpTopicsMerge.php:33: * @see \help_update_10200()
core/modules/help/tests/src/Functional/Update/HelpTopicsUninstall.php:32: * @see \help_update_10200()
Remove check and update hook help_update_10200() when updates for 11.0 will be prepared
patch, review, commit
no
no
no
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !8809
Comments
Comment #2
catchComment #3
andypostI think we can remove help topics module's remains within the same issue
Comment #4
andypostAlso when update hooks will get cut on next major, or it will need own own issue
Comment #5
quietone commentedStumbled upon this. Issues for removing deprecated code should be a child of the relevant Meta issue to remove deprecated code from the upcoming major.
Comment #7
andypostAdded review, only this 2 places to fix but MR removing code instead of just version check
Comment #8
quietone commented@andypost, thanks. I did this quickly without much thought or knowledge of the help and search system. I need to concentrate on other things for a while so hopefully someone else can finish this.
Comment #9
andypostquickly reverted and fixed, looks it can be backported to 10.3.x
Comment #10
andypostNo, it should not be backported to 10.x because the hook is required to be executed before cache cleaning
The fix probably needs better comment about order of cache cleaning for plugins, somehow search is cleaned after help, probably because of alphabetic order
Comment #11
andypostHistorically the hook creating new database table for search indexing but table-exists-check is too expensive to use at runtime
Comment #12
smustgrave commentedThis was posted in #needs-review-queue-initative so taking a look.
Maybe in the future we should drop a comment like //remove in drupal:11.0.0 so it gets picked up in greps.
But nice catch! Replacement seems fine
Comment #15
catchCommitted/pushed to 11.x and cherry-picked to 11.0.x, thanks!