Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DependencyInjection][Routing][Serializer][Validator] Deprecate XML configuration format#60568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Conversation
2169ea4 toc5c98cbComparec5c98cb toe7849d9Comparef21defc tod725030CompareLet's wait on the discussion on the issue to know what should actually be deprecated. This PR deprecates a lot more than what was mentioned in the issue it fixes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🚀
Some test cases need some love.
| publicfunctiontestInlineServicesAreNotCandidates() | ||
| { | ||
| $this->expectUserDeprecationMessage('Since symfony/dependency-injection 7.4: XML configuration format is deprecated, use YAML or PHP instead.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
shouldn't we rewrite this to use another format instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This test looks like it’s XML specific:#24491
| @@ -32,6 +33,8 @@ | |||
| class XmlDumperTestextends TestCase | |||
nicolas-grekasMay 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
side question: do we want to keep the xml dumper?
it's used by some external tools AFAIK, so likely yes
MatTheCatMay 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We also have commands loading such XML dumps, likedebug:container. Not sure what we should do about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
FTR I gave a try on other formats: PHP dumper cannot dump a non-compiled container, and YAML dumper suffers from#60573
| public function testImportWithGlobPattern() | ||
| { | ||
| $this->expectUserDeprecationMessage('Since symfony/dependency-injection 7.4: XML configuration format is deprecated, use YAML or PHP instead.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
maybe remove/spli the xml part? a legacy test case is usually planned for removal, which isn't the case for this one, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I assumed we could simply remove the XML part from the test on 8.0 but after a second look it seems it would be better to replace the XML config with a PHP one in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done
| @@ -116,8 +116,13 @@ public function testLoadParameters() | |||
| $this->assertEquals(['foo' => 'bar', 'mixedcase' => ['MixedCaseKey' => 'value'], 'values' => [true, false, 0, 1000.3, \PHP_INT_MAX], 'bar' => 'foo', 'escape' => '@escapeme', 'foo_bar' => new Reference('foo_bar')], $container->getParameterBag()->all(), '->load() converts YAML keys to lowercase'); | |||
| } | |||
| /** | |||
| * @group legacy | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
same concern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Addressed as well
| DependencyInjection | ||
| ------------------- | ||
| * Deprecate XML configuration format, use YAML or PHP instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
genuine question; wdyt documenting PHP before YAML?
seeing more and more config file written in PHP lately as the ecosystem and tooling evolve
(symfony core included)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yaml is still the best way to go in combination with flex/recipes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If we’re going to order the alternatives, is it correct for the other components’?
d725030 tod1a9099Compared1a9099 toded957cCompare
Uh oh!
There was an error while loading.Please reload this page.
From#60560 (comment)