Create hook_runtime_requirements

Think the CR should be updated with some code examples

Also some test coverage for the hooks to show they work.

Updated both.

Didn't push up the alter test.

I don't think we need to convert all here but could we convert least 1-2 instances to use this new hook. With the test coverage I do believe it's working but it being used live would be great.

Appears to have a pipeline issue. But keeping an eye out for this one.

Thanks for adding that example!

Rest looks good to me

Thanks for this! Exciting.

I’m only looking at the MR on my phone. I have some concerns. I’ll post a thorough review later when I’m not thumb-typing.😅 But I don’t believe this is totally ready in its current state. No rush, since this isn’t happening for 11.1.x. But getting out of the RTBC queue for now so committers don’t waste time on it.

Or I guess to make @smustgrave happy, NW is better. 😂 Sorry to change the status to that without explanation, but I’ll open MR threads later with details.

I'm not sure about the decisions that are creating this API. It feels like the question is how can we make this an OOP hook - and not what is the correct API for gathering requirements during install, update and runtime. The solution being advocated for in this and the other issues feels just asking ourselves what is possible with OOP hooks - and not what it is the correct API for gathering requirements from modules.

I'll reply on the meta.

Marking this postponed since an alternate approach was suggested in the Meta

The MR thinks the merge is blocked due to conflicts. Would you be willing to rebase? I just did a thorough review on the sister issue. Probably many of those nits would apply here, too. ;) I'm sadly out of contrib time for today, so I'll have to come back to this later for a more thorough review of the details here.

Thanks! I can rebase and transfer the feedback here too tomorrow.

I applied all feedback from 📌 Create class to replace hook_update_requirements Active

Believe after this rebase this one is good to go.

Re-ran the pipeline and all failures were random.

CR reads well with the examples and conversion in the file module believe shows it works in real time.

Needs rebase after 📌 Add array return to all hook_requirements implementations Active landed. Also, added a bunch of suggestions to cleanup the API docs.

Initial pass at saving credit: @nicxvan for all the work, and @smustgrave and myself for reviews.

I think the failure is from upstream, nothing here would make the navigation stylesheet larger.

I have a very minor lingering concern that INFO and OK are weird for update. They’re never displayed unless there’s an error or warning, in which case all update requirements, even OK and INFO, are shown to the user, distracting them from the thing they actually have to fix. 😕 This is not a problem to be solved in this issue. 😅 Maybe some of it can be addressed at 📌 Deprecate REQUIREMENT_INFO in favor of REQUIREMENT_OK Active , but probably we’ll want yet another follow up for the UX of update.php when there are requirements problems.

That said, I think we’ve done everything we can to make this a clean and solid improvement. RTBC!

Thanks,
-Derek

This, too, might be RTBC, but lemme do it right if true.

Yeah, I don’t know if the quick-fix here in an unrelated test is the right approach. I believe we should postpone this on resolving 📌 Add headroom to the navigation performance test Active

Blocker is in. This needs a rebase and to revert the last commit.

Yep I'll take care of it

Phew! Thanks for sticking with this through all the rounds of reviews. Apologies I didn't spot everything all at once.

Everything is ready here now. Pipeline is green. Code and docs have been polished so much they shine. 😂 No other improvements I can spot. Per the meta, these separate hooks really are the right approach for splitting up the beast of hook_requirements().

Ready To Be Committed. 😉

p.s. Did a light edit on the CR for formatting and pedantic nits. 😂 https://www.drupal.org/node/3490851/revisions/view/13843726/13854897 →

Committed 5e0a4ce and pushed to 11.x. Thanks!

quietone → closed merge request !10410

Automatically closed - issue fixed for 2 weeks with no activity.