Convert hook_requirements that do interact with install time that are not system

I am still working on this.

I am still working through this.

Not sure I'm following the need to break up into 2 file locations now?

When it's for phase runtime or update they are hooks still so those go in the Hook namespace.
If they are install time then that is not a hook, it is a special class, the change record is here: https://www.drupal.org/node/3492429 →

Gotcha

Mostly looks good, thanks! A few nits.

Applied almost all of them, I probably would have left most of those to follow ups since copy and paste issues like this are already hard enough to review.

I left one I didn't think was correct.

Sorry, my 😉 about adding StringTranslationTrait was shorthand for "add this, then use it". But you only added, so phpcs is complaining about unused use. Can either revert that suggestion, or actually use the trait...

Thanks,
-Derek

Ah sorry, for some reason I was looking at the wrong class you removed the extra return, I applied that locally.

I also removed the string translation, I saw the other translation change and thought it was the complete suggestion.

I decided to remove it since this is part of the install time, and it's better to handle those updates in a follow up I think. For the bits that are outside of install time I think it's ok to add injection here.

Cool, thanks!

New day, fresh eyes, new MR threads. 😂

Created the follow ups for the ones I think are out of scope.
Addressed other comments.

Ready for review again!

I'll keep an eye on tests.

Thanks for those follow-ups. Resolved all the open threads with links. Re-reviewing now.

With all the follow-ups open, all my concerns have been addressed. I see no way to further improve this MR that's in scope. The pipelines are all green. RTBC!

Thanks,
-Derek

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Appears to be random nightwatch failure, re-running is fine

Restoring previous RTBC status.

One comment about duplication

Replied re duplication. Back to RTBC.

Sorry, wrong link. See related issues.
📌 [pp-1] Review requirements duplication Postponed

larowlan → closed merge request !11504

Committed to 11.x and unpostponed the follow up

Thanks

Thanks!

Giving this its proper parent issue for posterity...

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