Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[FrameworkBundle] Deprecate not setting some options (uid, validation)#51357
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
[FrameworkBundle] Deprecate not setting some options (uid, validation)#51357
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| ->validate() | ||
| ->always(function (array $v): array { | ||
| if (empty($v['password_hashers'])) { | ||
| trigger_deprecation('symfony/framework-bundle', '6.4', 'Not setting the "security.password_hashers" config option is deprecated. It will default to "Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: auto" in 7.0.'); |
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'm not sure about this deprecation. Default values for a prototyped array node are not merged with provided config. So as soon as any config file (or config prepended by a bundle) adds some other config for password hashers, the config for PasswordAuthenticatedUserInterface would be lost.
I'd suggest keeping that one in the recipe.
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.
They would be merged if we didn't callperformNoDeepMerging, isn't it?
Why don't we perform deep merging BTW?
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.
Anyway, let's drop this, it's not needed to deprecate not setting this
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.
🤷a5cfc22 "some bug fixes in DIC config classes"
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.
@nicolas-grekas no. deep merging is about the various configs received as input, not the defaults
d1c71de toab2faa1CompareThere 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 addressed@stof's review.
There are remaning deprecations, please fix them.
ab2faa1 to878cda2Compare…tions (Jean-Beru)This PR was merged into the 6.4 branch.Discussion----------[FrameworkBundle] Deprecate not setting some options| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | no| New feature? | no| Deprecations? | yes| Tickets |Fixsymfony#51097| License | MIT| Doc PR |This PR deprecates not setting 5 options in framework configuration to match the v7.0 recipe:* `handle_all_throwables` which is `false` by default and is set to `true` by recipe* `php_errors.log` which use `$debug` value by default and is set to `true` by recipe* `session.cookie_secure` (if session is enabled) which is `null` by default and is set to `auto` by recipe* `session.cookie_samesite` (if session is enabled) which is `null` by default and is set to `lax` by recipe* `session.handler_id` (if session is enabled) which is `session.handler.native_file` by default and is set to `null` by recipe (could be `session.handler.native_file` if `framework.session.save_path` is set)Seesymfony#51097.Commits-------7f9998e [FrameworkBundle] Deprecate not setting some options
878cda2 toaa3fbc5CompareThank you@Jean-Beru. |
…daubois)This PR was merged into the 7.0 branch.Discussion----------[FrameworkBundle] Remove config deprecations| Q | A| ------------- | ---| Branch? | 7.0| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets | -| License | MIT| Doc PR | TodoFollow-up of:-#51325-#51357Commits-------4fe6f5b [FrameworkBundle] Remove config deprecations
Uh oh!
There was an error while loading.Please reload this page.
This PR deprecates not setting 4 options in framework configuration to match the v7.0 recipe:
uid.default_uuid_versionwhich is6by default and is set to7by recipeuid.time_based_uuid_versionwhich is6by default and is set to7by recipevalidation.email_validation_modewhich isnullby default and is set tohtml5by recipeSee#51097.