Contrib.social

Make RecipeExtensionConfigStorage's constructor less confusing

Open on Drupal.org →
Created on 23 February 2024, 4 months ago
Updated 12 March 2024, 4 months ago

Problem/Motivation

In https://git.drupalcode.org/project/distributions_recipes/-/merge_request..., Wim noticed that the $configNames parameter of \Drupal\Core\Recipe\RecipeExtensionConfigStorage::__construct() is confusing -- it's very odd to say "an empty array means everything!"

Proposed resolution

Two options suggest themselves here.

  • We could change the type hint to ?array, where NULL means "everything". This isn't a whole lot better, but it's at least got precendent in core -- that's exactly how \Drupal\Core\Entity\EntityStorageInterface::loadMultiple() works.
  • We could change it to array|bool, with intent to change that to array|true once we require PHP 8.2 (which introduced the standalone true type), where TRUE means "everything".
📌 Task
Status

Needs work

Version

11.0

Component

Code

Created by

🇺🇸United States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
  • API clean-up

    Refactors an existing API or subsystem for consistency, performance, modularization, flexibility, third-party integration, etc. May imply an API change. Frequently used during the Code Slush phase of the release cycle.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @phenaproxima
  • First commit to issue fork.
  • Merge request !80Suggested change → (Open) created by narendraR
  • Pipeline finished with Failed
    4 months ago
    Total: 206s
    #103836
  • Pipeline finished with Failed
    4 months ago
    Total: 127s
    #103854
  • Pipeline finished with Success
    4 months ago
    Total: 151s
    #103906
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇮🇳India narendraR Jaipur, India
  • Status changed to Needs work 4 months ago
  • Comment 4 months ago →
    🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
  • Pipeline finished with Failed
    4 months ago
    Total: 132s
    #104263
  • Pipeline finished with Failed
    4 months ago
    Total: 133s
    #104280
  • Pipeline finished with Success
    4 months ago
    #104305
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇮🇳India narendraR Jaipur, India
  • Status changed to Needs work 4 months ago
  • Comment 4 months ago →
    🇺🇸United States phenaproxima Massachusetts

    Ideally we could remove the multiple is_array() calls - we shouldn't be changing runtime logic just to shut PHPStan up, if we can at all avoid it. Posted a suggestion, but if that won't work, we can keep it as-is.

  • Pipeline finished with Failed
    4 months ago
    Total: 132s
    #104636
  • Pipeline finished with Failed
    4 months ago
    Total: 167s
    #116974
  • Comment 4 months ago →
    🇮🇳India narendraR Jaipur, India

    Re #7, Test started failing after reverting the changes.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024