hook_requirements() doesn't say that severity is optional, or what the default is

In general, I disagree with postponing a documentation issue on a decision that will be made in the future. I think the documentation should reflect the current state of the code. Plus the other issue has not had an update to the patch there in 9 years. I see no gain in waiting.

Therefor I am un-postponing.

Assuming I haven't misunderstood, this should be ready for review. Please see MR !9498.

I've added the (optional) before severity in the docblock.

Seems straight forward.

A parameter marked 'optional' need to say what the default will be.

Hey @joachim,

How should i mark the default? Would it be (default) before the option? Does the default need to be the first in the list? (default) at the end of the item? I couldn't find anything in the coding standards, unsure if maybe i've missed it.

Thanks

Add something like "If omitted [say what happens]."

Hi, MR !9498 should now be ready for review.

Thanks

Small suggestion to simplify it

Hopefully i understood #31 correctly. Added the small change! Please review

Tweaked slightly in the editor but looks good to me.

I see now @smustgrave. Cheers and apologies!

No worries at all! Thanks!

I read this issue, the MR and the related issue. I think this is an improvement. Leaving at RTBC

Actually, I am mistaken. A change is needed here. The default value is REQUIREMENT_INFO, see

• #665790-291: Redesign the status report page →
• https://git.drupalcode.org/project/drupal/-/blame/11.x/core/modules/syst...

As per the comment, I made a minor change to the default value in the comment section.

LGTM.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

arunkumark → changed the visibility of the branch 2982582-hookrequirements-doesnt-say to hidden.

Created MR against 11.0.x, Now able to review.

Why open a new branch when the previous was pointing to the correct target?

arunkumark → changed the visibility of the branch 2982582-hookrequirements-doesnt-say to active.

arunkumark → changed the visibility of the branch 2982582-hookrequirements-doesnt-say-d11 to hidden.

I faced the issue of creating MR against the 11.x branch. Now it is working to update the MR

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

MR needs a small tweak, but is nearly there!

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

anish.a → made their first commit to this issue’s fork.

Adding for review again after rebase.

There are test failures. I am unsure about how to fix it.

Re #37 🐛 hook_requirements() doesn't say that severity is optional, or what the default is RTBC :

Actually, I am mistaken. A change is needed here. The default value is REQUIREMENT_INFO, see

#665790-291: Redesign the status report page
https://git.drupalcode.org/project/drupal/-/blame/11.x/core/modules/syst...

The linked code still matches what was mentioned in #2 🐛 hook_requirements() doesn't say that severity is optional, or what the default is RTBC , and should probably be documented as such:

      $severity = $severities[REQUIREMENT_INFO];
      if (isset($requirement['severity'])) {
        $severity = $severities[(int) $requirement['severity']];
      }
      elseif (defined('MAINTENANCE_MODE') && MAINTENANCE_MODE == 'install') {
        $severity = $severities[REQUIREMENT_OK];
      }

Which sounds like undefined severities default to REQUIREMENT_OK while installing, and REQUIREMENT_INFO otherwise.

This should indeed be documented explicitly.

> Which sounds like undefined severities default to REQUIREMENT_OK while installing, and REQUIREMENT_INFO otherwise.

Yup, well spotted!

(Also, the logic in that piece of code could be a lot clearer - I'll file an issue to refactor it.)

(Also, it seems like REQUIREMENT_INFO might only apply during runtime? Again, something for a separate issue.)

An unrealted test was failing, but after a rerun, its all green now. Marking as needs review unless i've missed something else?

lgtm

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

Some change upstream must have affected the PHPStan check, because the bot failed on this:

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.


 [ERROR] No files found to analyse.



PHPStan: failed

----------------------------------------------------------------------------------------------------

back to rtbc

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

Another issue with phpstan. has less information than #61

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.

PHPStan: failed

----------------------------------------------------------------------------------------------------

I am unsure how to fix this

Seems like a false positive moving to NR.