Problem/Motivation
Follow-up from #3494908: Set skip procedural scanning for all modules in core. Let's make the changes we need to make to mark the remaining modules.
Modules that implement hooks that cannot be converted we use the #[StopProceduralScan] Attribute.
This is for update.
Steps to reproduce
N/A
Proposed resolution
Add #[StopProceduralScan] Attribute to modules that have hook_requirements, hook_hook_info or hook_module_implements_alter
Move procedural hooks to top of files that must remain
If there is not already a services file create it
Remaining tasks
N/A
User interface changes
N/A
Introduced terminology
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|
Issue fork drupal-3495506
Show commands
Start 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:
- 3495506-update-move-procedural
changes, plain diff MR !10673
Comments
Comment #2
deepali sardana commentedComment #3
deepali sardana commentedI have created the patch please review it and let me know
Comment #4
nicxvan commentedWe no longer use patches for core please convert your patch to an MR.
Comment #5
nicxvan commentedAlso this is for the update module not node.
Further, we only need that attribute on the first non hook function.
And there are changes unrelated to this.
Comment #6
nicxvan commentedComment #7
nicxvan commentedComment #8
nicxvan commentedI am working on this.
Comment #10
nicxvan commentedComment #11
bramdriesenLooks straightforward enough to me. RTBC
@deepali sardana, please stop making patches. You’ve been told this on like 10 issues already. Yet every new issue you start working on you create a patch.
Comment #12
dwwIt doesn't look like we're actually converting anything here, right? We're just adding the attribute for when to stop scanning for procedural hooks in a bunch of files. Do we actually want to do some conversion, instead? Isn't that part of the fun of having split these out into smaller issues so we can make bigger changes where needed?
Comment #13
nicxvan commentedI'm not sure what you mean, all of the hooks that can be converted have been.
Unless you mean the non hooks, then I think that's a separate issue.
Comment #14
dwwSorry, totally missed. I had a stale copy of a branch checked out, and was only looking at the MR diff which seemed thin for some reason.
So we've already moved everything we can move, and converted everything to UpdateHooks, etc. The title here is kinda misleading. Maybe this is closer? I know you opened these issues in bulk with copy/pasta summaries and titles, but I think it's worth refining before it ends up in the Git history.
p.s. Saving credit to nicxvan for the MR, bramdriesen and myself for reviews, but not the patch in #3.
Comment #15
nicxvan commentedThanks! Yeah this one was separate cause there were so many .inc files I wanted to take more time to review. Locale was in the same bucket.
Comment #16
larowlanCommitted to 11.x - thanks!