Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC | Lets switch configs from xml to php #1859

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Conversation

gassan
Copy link
Contributor

@gassan gassan commented Dec 18, 2021

Symfony rewritten configs for all own packages to php. it is done here for hwi-oauth too.

Some links: https://tomasvotruba.com/blog/2020/07/16/10-cool-features-you-get-after-switching-from-yaml-to-php-configs/

PS: Old files not yet removed so that you can compare them before merge.

@gassan gassan force-pushed the configs-to-php branch 3 times, most recently from 085a14d to 344271f Compare December 19, 2021 01:07
@gassan
Copy link
Contributor Author

gassan commented Dec 19, 2021

I think the last commit must be reverted. phpstan moans about security_v5.php (SecurityConfig not exists), old yaml testapp configs are more cleaner.

@gassan
Copy link
Contributor Author

gassan commented Dec 20, 2021

Cleaned.
I promise to switch tests/App configs to php as soon as symfony delivers its own app config recipes in php. :)
Let's leave the tests/App config files as yaml for now.

@stloyd
Copy link
Collaborator

stloyd commented Dec 20, 2021

Personally I would leave yaml files and recommend php ones, IMHO change is not really significant to force people to change ie routing files etc

Also there is needed change in documentation to recommend php ones as there is either xml or yaml.

@gassan gassan requested a review from stloyd December 21, 2021 14:17
@stloyd
Copy link
Collaborator

stloyd commented Dec 21, 2021

As said, IMHO we should revert yaml routing removal, other are not affecting end users.

After that we can reword changelog cause it will be no more BC break but deprecation.

@gassan gassan force-pushed the configs-to-php branch 2 times, most recently from fb88e5f to 6045247 Compare December 21, 2021 23:00
@gassan gassan changed the title RFC | Lets switch configs from xml/yaml to php RFC | Lets switch configs from xml to php Dec 21, 2021
@gassan
Copy link
Contributor Author

gassan commented Dec 21, 2021

I wrote whole time about yaml files, but there was no yaml configs in the bundle, all are xml :)

@stloyd stloyd merged commit dc8832f into hwi:master Dec 22, 2021
@stloyd
Copy link
Collaborator

stloyd commented Dec 22, 2021

Merged, thanks @gassan !

@gassan gassan deleted the configs-to-php branch December 22, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants