Skip to content

[Serializer] Ensure that groups are strings #17430

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

Closed
wants to merge 3 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Jan 18, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

@paradajozsef
Copy link
Contributor

👍

One more thing to note. If such a yml is provided to the loader:

Acme\MyObj:
    attributes:
        foo:
            groups: 'group1'

Invalid argument supplied for foreach() warning raises, because it is not ensured here.

If you think this should be in another PR, than sorry. :)

@dunglas
Copy link
Member Author

dunglas commented Jan 19, 2016

Good catch @paradajozsef, I've added a separate commit in this PR.

@paradajozsef
Copy link
Contributor

👍

@GuilhemN
Copy link
Contributor

This is more a bug fix than a new feature ;-)

👍 debugging is easier with this kind of check 😄

@xabbuh
Copy link
Member

xabbuh commented Jan 19, 2016

We should include the name of the file and probably some other related information (class name, property or something like that) to make it easier to spot the place where the configuration is wrong.

@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

I agree with @xabbuh

@dunglas
Copy link
Member Author

dunglas commented Jan 26, 2016

Status: Need Review

@xabbuh @fabpot done

@fabpot
Copy link
Member

fabpot commented Jan 26, 2016

Thank you @dunglas.

fabpot added a commit that referenced this pull request Jan 26, 2016
This PR was squashed before being merged into the 2.7 branch (closes #17430).

Discussion
----------

[Serializer] Ensure that groups are strings

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Commits
-------

0a3b877 [Serializer] Ensure that groups are strings
@fabpot fabpot closed this Jan 26, 2016
@dunglas dunglas deleted the sr_yaml_groups branch January 26, 2016 07:01
@fabpot fabpot mentioned this pull request Feb 3, 2016
This was referenced Feb 28, 2016
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.

6 participants