Move REQUIREMENT_* constants out of install.inc file

Created on 17 September 2017, over 7 years ago
Updated 19 March 2023, almost 2 years ago

Problem/Motivation

Follow-up to πŸ“Œ Warn site admins when composer dev dependencies are installed inside of docroot Needs work where we have a test that is a kernel test only because KernelTestBase makes REQUIREMENT_OK available to the system under test.

Let's move the REQUIREMENT_* family of constants to a loadable namespace.

@alexpott on #50 said:

I feel this is OO-ification for OO-ification's sake. We're only moving the constants so they can be autoloaded. But a proper redesign of the requirements system might end up with Requirement value objects that makes these constants redundant - imagine $requirement->isError() etc...

If what we want to solve autoloadability of constants there's another way. We could add core/constants.php to our autoloader in the files section and repeating myself from the other issue the advantages are

  • there's no deprecation
  • the constants become available to autoloaded code
  • as constants.php grows it becomes a place to learn about all the core provided constants

The one downside is this approach is only available to core and not core modules/themes, or contrib or custom because they don't using composer to set up their autoloading. But I'm not sure that outweighs the advantages listed.

Proposed resolution

TBD.

Remaining tasks

Make a patch, or decide this is a Won't Fix.

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Needs work

Version

10.1 ✨

Component
InstallΒ  β†’

Last updated about 21 hours ago

No maintainer
Created by

πŸ‡ΊπŸ‡ΈUnited States mile23 Seattle, WA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡«πŸ‡·France andypost

    Nowadays idea of discovery Requirement objects could use attributes to define title, value and description.
    Other similar kind of checks/tasks install system has, so approach from #50 sounds viable

    +++ b/core/lib/Drupal/Core/Render/Element/StatusReport.php
    @@ -35,13 +37,13 @@ public static function preRenderGroupRequirements($element) {
         $severities = static::getSeverities();
    ...
    -      $severity = $severities[REQUIREMENT_INFO];
    +      $severity = $severities[Requirements::SEVERITY_INFO];
    ...
    -        $severity = $severities[REQUIREMENT_OK];
    +        $severity = $severities[Requirements::SEVERITY_OK];
    ...
           $grouped_requirements[$severity['status']]['title'] = $severity['title'];
    
    +++ b/core/lib/Drupal/Core/Requirements.php
    @@ -0,0 +1,30 @@
    +final class Requirements {
    

    it could be enum but it actually used only to group requirements

  • πŸ‡ΊπŸ‡¦Ukraine voleger Ukraine, Rivne

    Seems like we have an issue that already has some work done in that way πŸ“Œ Add value objects to represent the return of hook_requirements Needs review

  • πŸ‡«πŸ‡·France andypost

    Looks yep!

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I am going to work on this because we need these constants for hook_update_requirements and hook_runtime_requirements.

    The one downside is this approach is only available to core and not core modules/themes, or contrib or custom because they don't using composer to set up their autoloading. But I'm not sure that outweighs the advantages listed.

    Since these classes are used in core, contrib, and custom, I think this disqualifies the constants.php approach.

    However, we don't actually have to deprecate these here, I think we can do that when we deprecate hook_requirements, for now we can just create the new classes and just use them as we convert hooks.

  • πŸ‡«πŸ‡·France andypost

    Deprecation suppose removal of the usage, 240LOC surely needs chunking

    git grep REQUIREMENT_ |wc -l
    241

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Yes, I wouldn't create the deprecation here, just create the new landing place for constants and the new constants we need.

    I wonder if there is a way to have both.

    E,g. something like
    /Drupal/Core/Constants
    That is where the constants live, you can use it if you need it.
    Maybe it makes sense to auto load it so they are available everywhere that uses autoloading.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I am not going to work on this, I think @berdir found a clean workaround, I think my comments about constants.php are still valid.

Production build 0.71.5 2024