Create class to replace hook_install_requirements

Makes sense to move this to a hard-coded class instead of a hard-coded function name, I don't there's anything else we can do in install.

However the MR is missing some hook_requirements() which affect mutliple phases, such as in package_manager_requirements()

That was considered, I'll update the meta to add more details, but here is the gist.

We replace the $phase = 'install' portion with this issue.

We replace the $phase = 'update' with 📌 Create class to replace hook_update_requirements Active

And we replace the $phase = 'runtime' with 📌 Create hook_runtime_requirements Active

Once we have all three this covers most cases, but for more complex hook_requirements like system and package_manager with multiple phases we convert in a follow up.

Once core is converted we deprecate hook_requirements being called without LegacyHook being attached.

> Makes sense to move this to a hard-coded class instead of a hard-coded function name, I don't there's anything else we can do in install.

The problem with these hooky-classes, like this and ModulePascalServiceProvider is that unlike hooks, we have no standard for documenting them.

We can document it on the interface, which is where I put it, it definitely needs some fleshing out, but it's there.

If you mean the implementation side then we can just document the classes implementing it.

That's good, but we'd still be losing the discoverability of it in the documentation as an API.

With hooks, there is a @hooks topic that explains the concept and lists the API elements. With HookyClasses, you only learn of a particular class if you happen to stumble on it.

Oops, didn't mean to change status.

Though I'm confused why the other two replacements of this hook are hooks, but this one is a class? What's the difference?

I still think 18 is a separate issue and not a blocker. For 19 there are two reasons:
1 the other two are invoked by module handler so we can just change the hook.
2 this one happens during Drupal or module install is to early.

This is ready to review again, the concerns on the meta have been resolved.

> I still think 18 is a separate issue and not a blocker.

Every time we say this we are eroding the quality of our documentation. Each time it's a small separate issue and not a blocker, but these keep adding up.

I'm happy to work on wording in a sibling issue if you create it, I think it would belong in a subpage here: https://www.drupal.org/docs/getting-started/system-requirements →

Applied some nitipicky stuff but reviewed the rest of the changes and nothing seems off to me.

Thanks for the CR updates we mentioned in slack.

I did open 📌 How to best handle documentation around new OOP hooks Active as a child of the META and tagged it as a 11.2 release priority.

I think we should not be doing this. Creating another way to load code from a module that is not installed feels like we are perpetuating the problems of hook_requirements() with the install phase.

Added some comments to the MR. But I think we should explore reading the modules composer file and check that instead of doing this.

Those are good ideas I can update this later.

I still think we should do this for the reasons stated in in the meta, particularly it's easier for people to know each has a phase somewhere to go. We can mention in the CR that it is preferred to use composer and a link to the issue to watch, and that this is intended as a bridge.

Ok I addressed all of your feedback I agree it looks cleaner.

I did copy the class load from the drupal_check_profile, is that a separate concern?

Setting it back to needs review because I addressed the feedback and it's green.

I'm going to update my comment on the meta.

> I think it would belong in a subpage here: https://www.drupal.org/docs/getting-started/system-requirements →

Putting some documentation on d.org is great, but that's still a worsening of what we have. Hooks are in-code, and most importantly, *systematic*.

You look at this:

 * Implements hook_requirements().

and hook_requirements() is obvious as a thing that you can look for. API module handles it automatically to link up the specific instance to the general documentation for the component.

You go to the hook_requirements() documentation, and you see it's in the @hooks group. API module makes this a link.

You go to the @hooks group topic, and that explains what hooks are as a type of extension point. API module lists all of the hooks that exist.

But with something like:

class MyModuleServiceProvider

we have currently at best an interface. It doesn't tell you the class must be magically name, and it doesn't connect to the other extension points that behave in the same way.

As a counterpoint, I think it is far more standard to look at the interface something is extending and have the documentation there.

That documentation tells you that it must be magically named.

The implements piece is something the community is used to but you either have to leave the ide to find docs or you have to know to search for function hook_requirements otherwise search results are flooded with other implementations.

Also implements comments can be wrong or missing, extend can't be.

Again finding other implementations of a hook either requires leaving your ide, knowing regex, or the implementors perfectly following the naming scheme for a comment.

Finding classes implementing an interface is mostly built in to ides and something people beyond the community are used to too.

I agree having docs outside the ide is helpful which is why I suggested that docs pages, other suggestions are welcome.

If you're looking for a way to document classes implementing an interface the issue smustgrave created is probably the place to discuss that process and what is needed. It should probably be moved to api and retitled to focus on this use case since we already have one for hooks specifically.

@alexpott, I addressed your feedback and created the followup here: 📌 Remove class_loader in drupal_check_profile Active .

I agree that it would be nice if we can get rid of most* install requirements hooks but that requires multiple new features (other modules, extensions, composer dependencies, conflicts), not all of which can be composer (we can't use composer conflict because we can not and don't want to prevent them being present, it's about them being installed or not).

hook_requirements() is the last remaining "regular" hook (there's just preprocess and special install/update stuff then). If we have that and preprocess done we can drop that stop parsing attribute and so on. modules are either fully converted or not, no weird middle ground.

In other words, it would be very nice to find a way to deal with it. I was thinking we could just keep hook_requirements with phase install for now until we see what's left once we improve other things. But I think that's hard to deprecate and detect allowed implementations vs not, we can't introspect what $phase it has and what it does with it.

On the class name: Do we really need to prefix it with a camel-cased moduel name? Yes, we do it for ServiceProvider, but it's namespaced anyway. Yes, we'd have 5,10, ... "InstallRequirements" classes, but is that really a problem?

That's the only part I agree with from #31. Everyhing else I very much don't. As #32 said, what #31 describes is like 4 different Drupalisms that nobody else uses and new developers don't know about. Nobody except Drupal uses a .install file. Nobody else has an api module, api.drupal.org only provides info on core, it requires a Drupal-aware IDE like PHPStorm or knowing what to search for to maybe find the hook_requirements() documentation and understanding the *magically named* hook functions. Now you need to look at src/ (where _all_ code will be soon enough), where there might be an Install folder and then a Requirements class inside. Looking up the interface is standard PHP stuff, documentation on that is standard PHP stuff. To me, that is way better DX, especially for developers that don't know Drupal. And we want and must attract new developers.

That said, if we're not sure yet if this has a future, we could also just rename it to hook_install_requirements(), but then we have to explain that this is very different from the other two split requirements hooks. I'd prefer a class but I don't care too much. What I do care about is being able to forget about as soon as possible.

On the class name: Do we really need to prefix it with a camel-cased moduel name? Yes, we do it for ServiceProvider, but it's namespaced anyway. Yes, we'd have 5,10, ... "InstallRequirements" classes, but is that really a problem?

Yes, we cannot autoload it so there is no way to find it without knowing the naming convention unless you have a different idea.

> Yes, we cannot autoload it so there is no way to find it without knowing the naming convention unless you have a different idea.

My naming convention suggestion is only and always name it "InstallRequirements", nothing dynamic. It's namespaced, it doesn't need to have a prefix. Downside is that there will be a bunch of classes with that name, but the long term plan is removing all/most of them.