Skip to content

Conversation

@sroze
Copy link
Contributor

@sroze sroze commented Apr 3, 2018

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ø
License MIT

This test case wasn't updated after a change in the PR to automatically enable the validation middleware based on the interface.

),
'validation' => array(
'enabled' => false,
'enabled' => !class_exists(FullStack::class) && class_exists(Validation::class),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only be !class_exists(FullStack::class) as for other configurations. Which makes sense as the Validator component is always available in tests, being part of the require-dev section :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe. I was pushing this change. Done 🙃

@sroze sroze force-pushed the fix-validation-configuration branch 2 times, most recently from 364288c to 4486734 Compare April 3, 2018 21:33
@sroze sroze force-pushed the fix-validation-configuration branch from 4486734 to 235e037 Compare April 3, 2018 21:36
@fabpot
Copy link
Member

fabpot commented Apr 4, 2018

Thank you @sroze.

@fabpot fabpot merged commit 235e037 into symfony:master Apr 4, 2018
fabpot added a commit that referenced this pull request Apr 4, 2018
…est case (sroze)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[FrameworkBundle] Fix validation configuration default test case

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ø
| License       | MIT

This test case wasn't updated after a change in the PR to automatically enable the validation middleware based on the interface.

Commits
-------

235e037 Fix validation configuration default test case
@Nyholm
Copy link
Member

Nyholm commented Apr 4, 2018

Thank you. Sorry for missing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants