[pp-1] Mark several more modules as converted

Glad to see the rest of this committed. Thanks for opening this as a new issue.

With last nights commits appears to need a rebase.

if you are another contributor eager to jump in, please allow the original poster @nicxvan at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Yeah I had left this in needs work because I knew this needed the test fixes in the other issue.

It's still 61 files +788 −683. Can/should we split this up further to be easier to review? Maybe media* and system their own issues, everything else in here? I think that'd get us closer to +/- 500 here.

I'll split media and library and then system to their own

Ok I split system and the media modules out, those two issues are ready for review too.

Can the IS be updated with exactly what's being achieved here? Example the child issues marked what modules are being converted so it was super clear.

Thanks.

I added a list of the modules that I updated to the IS. Did you want more detail such as which changes are for each module?

Thanks for your progress and effort @nicxvan for MR !10614. Looking forward for its release.

Did you want more detail such as which changes are for each module?

- no that's probably not needed, just needed a quick glance on what was done "several modules" was kinda vague.

Will try and find time this week to take a look, much larger then the others so takes a little longer.

Let's postpone this for a bit on 📌 Handle module preprocess functions Active

Blocker is in

Let's convert this to a meta, We have to re-evaluate some modules after 📌 Handle module preprocess functions Active .

We can also do this in groups and stages, we pushed this through faster originally due to the massive memory leak.

Ok I did a full audit of core modules, I checked whether the services parameter was set, if there were hooks that hadn't been converted, preprocess, template_preprocess, .inc, files and helpers.

I think this round is focused on hooks, preprocess and template preprocess in most cases. I think we only tackle .inc if they are preprocess hooks in them.

I think we try to balance how many are in a given issue but we address organization and what we can in each.

It ends up being a lot of changes even for mostly converted modules.

I think we may need to restrict this to an issue per module, I think we also want to isolate theme, theme suggestion and preprocess functions that do not inject services from ones that do.

I'm mostly through bigpipe and block on the issue linked.
Could use a high level review.

We seem to have multiple overlapping (meta) issues now. The block issue is now essentially 📌 [pp-1] Mark several more modules as converted Active and also does DI, regroups hooks and so on. And there's the template_preprocess meta issue.

I'm unsure what the best way to split these is. The way it's done for block now kind of means we could just inline the actual last step that this aims to do (add the flag to services.yml once ready) into the cleanup meta and close this. I think the performance gains for those last few remaining modules is minor to tiny and it's no longer a priority from a performance perspective.

That said, the cleanup is fairly low prio and I expect those issues to land only slowly as they are hard to review and get committed. IMHO it's more useful to make sure that we've converted all legacy hooks and also preprocess stuff, with the goal of being able to explicitly deprecate legacy hooks and preprocess stuff in 11.3 for D13. For preprocess, we could expand the scope of 📌 [pp-1] Convert Template Preprocess hooks to OOP equivalent Postponed: needs info to not only cover template_preprocess but all preprocess in modules. And I'd probably approach the hooks similar to 📌 Triggering deprecations for plugins using annotations when core plugin type has been converted to attributes Needs work . Figure out how exactly we want to deprecate legacy hooks, that will tell us which bits we missed, and then we can possibly have a single issue to convert those.

Yeah, as I was doing it I knew the scope was too large but I wanted to check for sure.

I think this one reverts back to the minimum needed to mark a module as converted worth two exceptions.
1 hook requirements are handled in the issues dedicated to those.
2 for smaller module's we include converting template preprocess.

Bigger modules like node, views, and system will need dedicated hook, proprietary and template issues.

Tests just started, but this should be our target, if there is a module like block_content that only has template_preprocess we can take care of it in that issue.