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] Fix init of YamlFileLoader::$classes for empty files #20828

Merged
merged 1 commit into from Dec 8, 2016

Conversation

Projects
None yet
3 participants
@nicolas-grekas
Member

nicolas-grekas commented Dec 8, 2016

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20761
License MIT
Doc PR -
@stof

This comment has been minimized.

Member

stof commented Dec 8, 2016

Would it be possible to write a test covering the actual bug (i.e. checking that getMappedClasses works) instead of asserting the internal property ? it would be more reliable (the class is free to do what it wants with its private property as long as the public API works fine)

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Dec 8, 2016

The property is protected, so it's a "public" side effect, which means the test is good :)

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Dec 8, 2016

@stof is this PR ok for you?

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Dec 8, 2016

Note that this PR is against 2.7. getMappedClasses exists only on 3.2

@stof

This comment has been minimized.

Member

stof commented Dec 8, 2016

👍
Status: reviewed

@nicolas-grekas nicolas-grekas merged commit 073a1da into symfony:2.7 Dec 8, 2016

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Dec 8, 2016

bug #20828 [Validator] Fix init of YamlFileLoader::$classes for empty…
… files (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[Validator] Fix init of YamlFileLoader::$classes for empty files

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20761
| License       | MIT
| Doc PR        | -

Commits
-------

073a1da [Validator] Fix init of YamlFileLoader::$classes for empty files

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:fix-empty-valid branch Dec 8, 2016

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