- Issue created by @srishtiiee
- First commit to issue fork.
- Status changed to Closed: duplicate
about 1 year ago 9:10am 28 March 2024 - ๐ฎ๐ณIndia srishtiiee
This will be covered in ๐ Add validation constraints to all system.* simple config (except system.rss) Needs work
- Status changed to Needs work
about 1 year ago 10:52am 16 April 2024 - Merge request !7516Resolve #3436096 "Add validation constraints to system.file" โ (Open) created by srishtiiee
- ๐ฎ๐ณIndia srishtiiee
srishtiiee โ changed the visibility of the branch 3436096-add-validation-constraints to hidden.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Tests are failing and I think we can fix that by just pushing the latest
origin/11.x
to this issue fork ๐ค - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
- ๐ฎ๐ณIndia yash.rode pune
yash.rode โ made their first commit to this issueโs fork.
- Status changed to Needs review
10 months ago 9:45am 1 July 2024 - Status changed to RTBC
10 months ago 1:47pm 1 July 2024 - ๐บ๐ธUnited States smustgrave
On a standard profile install on 11.x
Installed configuration inspector and applied the MRShows that the system.file is fully validated.
Checking the MR everything appears to have a return type
Believe this one is good to go.
- Status changed to Needs work
10 months ago 11:28pm 3 July 2024 - ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
If path is deprecated we should deprecate it. See https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... โ
Added some review comments.
- Status changed to Needs review
9 months ago 8:10am 9 July 2024 - Status changed to Needs work
9 months ago 3:32pm 9 July 2024 - Status changed to Needs review
9 months ago 7:07am 10 July 2024 - Status changed to RTBC
9 months ago 8:51am 15 July 2024 - Status changed to Needs work
9 months ago 12:23pm 17 July 2024 - ๐ง๐ชBelgium borisson_ Mechelen, ๐ง๐ช
I agree with catch in #19, moving this to Drupal\Core\Config seems like a better solution, let's do that.
- First commit to issue fork.
- ๐ณ๐ฑNetherlands bbrala Netherlands
Updated the class namespace as suggested, did do a rebase to clean up.
- ๐ง๐ชBelgium borisson_ Mechelen, ๐ง๐ช
Back to rtbc now that it is moved.
- ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
If we changing the 10.3 database dumps because of invalid schema this points to us needing an update function. I think we should not be changing the dumps.
Also I think we should be adding a new constraint that works like callback but uses the class resolver service to my more flexible.
- ๐ณ๐ฑNetherlands bbrala Netherlands
So, the path config key was deprecated in Drupal 8.8 it seems ( https://www.drupal.org/node/3039255 โ ). That kinda feels out of scope of this change, the fact is that it will need to go through deprecation? I don't even really see any usage in core, but that could be my search skills failing me.
But i don't feel we should do that here. Hopefully you agree.
- ๐ณ๐ฑNetherlands bbrala Netherlands
Waiting for build, since i had some cs issues ;x
- ๐ณ๐ฑNetherlands bbrala Netherlands
Adjusted it to actually do what was asked for.
Did revert the deprecation message for the path property. Would like to know if we need to make the change here, or if we should defer that. Seems like that has been here for quite a while. I do think we might be able to ignore it here, since it doesnt seem to be used.
- ๐ณ๐ฑNetherlands bbrala Netherlands
๐ Remove system.file.path config from system.schema.yml Active created to do the deprecation and upgrade path
- ๐ณ๐ฑNetherlands bbrala Netherlands
Pipe is all green after the upgrade stuff in the child. Think we are there.
- ๐ณ๐ฑNetherlands bbrala Netherlands
๐ Remove system.file.path config from system.schema.yml Active was merged, rebased this one.
- ๐บ๐ธUnited States smustgrave
Using configuration inspector
Does the new constraint need a CR? I vote yes but don't want to hold it up on that.
Rest appears to be addressed
-
alexpott โ
committed 68b8b571 on 11.x
Issue #3436096 by yash.rode, bbrala, srishtiiee, naveenvalecha,...
-
alexpott โ
committed 68b8b571 on 11.x