-
-
Notifications
You must be signed in to change notification settings - Fork 476
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
[Validator] Enable auto mapping by default #664
Conversation
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.
Pull request passes validation.
I think we should add a docs PR to go along with this - it’s a really important change. |
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.
(@dunglas please create a doc issue at least)
Actually, it's already documented symfony/symfony-docs#11132 |
I like this feature! But it does have side effects. For example, in If a user doesn't have the "opt-out" annotation above That's the big WTF that we need to navigate around if we're going to enable this feature by default. |
Does it make sense to create a user system without validation? I mean you already need to check that the username is not taken, that the password isn’t empty, etc... isn’t it? Cannot the command throw if the Validator component isn’t installed? |
I was thinking about this as well. Probably we need to allow a Btw, there is another docs issue about this being automatically enabled, that still needs work. |
Btw, I'd like to merge this as soon as we can so that it "feels" like 4.4 and 5.0 consistently have this feature enabled. But, I think we need the MakerBundle tweak and docs first. I'm working as quickly as I can on those :). |
MakerBundle PR for But the feature currently doesn't work :/ - see symfony/symfony#34672 And (sorry!) I'm fighting back a little bit again on this feature - symfony/symfony#34860. |
(please rebase so that the new checks can run) |
Diff between recipe versionsThanks for the PR 😍 In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes. symfony/validator3.3 vs 4.1diff --git a/symfony/validator/4.1/config/packages/test/validator.yaml b/symfony/validator/4.1/config/packages/test/validator.yaml
new file mode 100644
index 0000000..b18c866
--- /dev/null
+++ b/symfony/validator/4.1/config/packages/test/validator.yaml
@@ -0,0 +1,4 @@
+framework:
+ validation:
+ # As of Symfony 4.3 you can disable the NotCompromisedPassword Validator
+ # not_compromised_password: false
diff --git a/symfony/validator/4.1/config/packages/validator.yaml b/symfony/validator/4.1/config/packages/validator.yaml
new file mode 100644
index 0000000..a695e1a
--- /dev/null
+++ b/symfony/validator/4.1/config/packages/validator.yaml
@@ -0,0 +1,3 @@
+framework:
+ validation:
+ email_validation_mode: html5
diff --git a/symfony/validator/3.3/manifest.json b/symfony/validator/4.1/manifest.json
index 2a250e2..57f78dc 100644
--- a/symfony/validator/3.3/manifest.json
+++ b/symfony/validator/4.1/manifest.json
@@ -1,3 +1,6 @@
{
+ "copy-from-recipe": {
+ "config/": "%CONFIG_DIR%/"
+ },
"aliases": ["validation"]
} 4.1 vs 4.3diff --git a/symfony/validator/4.1/config/packages/test/validator.yaml b/symfony/validator/4.3/config/packages/test/validator.yaml
index b18c866..1e5ab78 100644
--- a/symfony/validator/4.1/config/packages/test/validator.yaml
+++ b/symfony/validator/4.3/config/packages/test/validator.yaml
@@ -1,4 +1,3 @@
framework:
validation:
- # As of Symfony 4.3 you can disable the NotCompromisedPassword Validator
- # not_compromised_password: false
+ not_compromised_password: false
diff --git a/symfony/validator/4.1/config/packages/validator.yaml b/symfony/validator/4.3/config/packages/validator.yaml
index a695e1a..350786a 100644
--- a/symfony/validator/4.1/config/packages/validator.yaml
+++ b/symfony/validator/4.3/config/packages/validator.yaml
@@ -1,3 +1,8 @@
framework:
validation:
email_validation_mode: html5
+
+ # Enables validator auto-mapping support.
+ # For instance, basic validation constraints will be inferred from Doctrine's metadata.
+ #auto_mapping:
+ # App\Entity\: [] 4.3 vs 4.4diff --git a/symfony/validator/4.3/config/packages/validator.yaml b/symfony/validator/4.4/config/packages/validator.yaml
index 350786a..1affc68 100644
--- a/symfony/validator/4.3/config/packages/validator.yaml
+++ b/symfony/validator/4.4/config/packages/validator.yaml
@@ -4,5 +4,5 @@ framework:
# Enables validator auto-mapping support.
# For instance, basic validation constraints will be inferred from Doctrine's metadata.
- #auto_mapping:
- # App\Entity\: []
+ auto_mapping:
+ App\Entity\: [] |
@nicolas-grekas done. The failure looks unrelated, or am I missing something? |
e47d1eb
I'm closing this proposal as "won't merge" because after discussing about this in the Core Team, it was decided that it's better to not enable |
Now that symfony/symfony#32107 has been merged, auto mapping can be on again.