- Issue created by @phenaproxima
- First commit to issue fork.
- Status changed to Needs review
4 months ago 8:48am 14 February 2024 - 🇺🇸United States phenaproxima Massachusetts
Oh, I love the way this is looking!
I think w can harden it somewhat, and add more test coverage to be sure this really works as designed. :) Commented with some ideas.
- Status changed to Needs work
4 months ago 12:18pm 14 February 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Did a detailed review and provided some pointers.
This relates to #3400672: [PP-1] Robustly validate the structure of recipe.yml → , because this introduces a new way that would allow you to write invalid recipes.
- Status changed to Needs review
4 months ago 9:39am 16 February 2024 - 🇮🇳India narendraR Jaipur, India
This MR still has phpstan issues, but wanted to make sure if this is heading in right direction.
- Status changed to Needs work
4 months ago 10:50am 16 February 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Reviewed 🏓 Definitely heading in the right direction!
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Still needs more work — both @phenaproxima and I left remarks on what is missing/blocking RTBC.
- Status changed to Needs review
4 months ago 2:20am 21 February 2024 - 🇺🇸United States phenaproxima Massachusetts
Honestly, I'm not sure what else is needed here.
- Status changed to Needs work
4 months ago 11:03am 21 February 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Very close! 👏 I see at least one missing test, one unnecessary method and one accidental public API addition, so for that.
- Status changed to Needs review
4 months ago 3:34pm 22 February 2024 - 🇺🇸United States phenaproxima Massachusetts
I think it's ready for another look. All feedback resolved, except for one, which I'm not sure how to approach.
- Status changed to Needs work
4 months ago 3:15pm 23 February 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Close, but NW for
\Drupal\Core\Config\ConfigBase::validateName()and a test with a sample expression I provided that causes a PHP warning 🤓 - Status changed to Needs review
4 months ago 6:03pm 23 February 2024 - Status changed to RTBC
4 months ago 12:25pm 26 February 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Agreed we can question the wisdom of `ConfigBase::validateName()`, but that's not up to Recipes to change. Recipes should be able to install any config that is valid. So let's open a core issue to tighten that instead? :blush:
@chx requested in #1701014-63: Validate config object names → to disallow
|and a range of other characters (but not(or)) and @xjm implemented it in #1701014-73: Validate config object names → … but forgot|. So AFAICT the best course of action is to create a follow-up to tighten this in Drupal core 👍 - First commit to issue fork.
-
alexpott →
committed 0954af94 on 11.x authored by
narendraR →
Issue #3420209 by phenaproxima, narendraR, Wim Leers: Allow config...
-
alexpott →
committed 0954af94 on 11.x authored by
narendraR →
- Status changed to Fixed
4 months ago 12:49am 28 February 2024 - 🇬🇧United Kingdom alexpott 🇪🇺🌍
Can someone file an issue against the documentation branch 1.x and add some docs from the issue summary about this. Thanks!
-
alexpott →
committed bc2542b5 on 10.2.x authored by
narendraR →
Issue #3420209 by phenaproxima, narendraR, Wim Leers: Allow config...
-
alexpott →
committed bc2542b5 on 10.2.x authored by
narendraR →
- 49287cc9 committed on patch
Update recipe 11.x patch 0954af94 Issue #3420209 by phenaproxima,...
- 49287cc9 committed on patch
- 536bec39 committed on patch
Update recipe 10.2.x patch bc2542b5 Issue #3420209 by phenaproxima,...
- 536bec39 committed on patch
- Status changed to Downport
4 months ago 9:04am 28 February 2024 - 🇮🇳India narendraR Jaipur, India
Created https://www.drupal.org/project/drupal/issues/3424296 🐛 Disallow |, ( and ) in config object names Active and https://www.drupal.org/project/distributions_recipes/issues/3424293 →
- Status changed to Fixed
4 months ago 10:05am 28 February 2024 Automatically closed - issue fixed for 2 weeks with no activity.
