Skip to content

'yml' is not a format and shouldn't be visible as constant from 'YamlEncoder' #28830

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

Merged
merged 1 commit into from
Oct 12, 2018

Conversation

mcsky
Copy link

@mcsky mcsky commented Oct 12, 2018

A little fix of my previous PR

This PR changes the constant visibility of the yml format as private.

Because as @stof mentionned yml isn't a format, so the constant shoudn't be public.

Otherwise, this will be confusing while using autocomplete, you see two formats. The user can ask himself if there is a difference between yaml / yml.

No need of that :)

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
License MIT
Doc PR

@fabpot
Copy link
Member

fabpot commented Oct 12, 2018

Thank you @mcsky.

@fabpot fabpot merged commit 674b359 into symfony:master Oct 12, 2018
fabpot added a commit that referenced this pull request Oct 12, 2018
…nt from 'YamlEncoder' (kevin-biig)

This PR was merged into the 4.2-dev branch.

Discussion
----------

'yml' is not a format and shouldn't be visible as constant from 'YamlEncoder'

A little fix of my [previous PR](#28815)

This PR changes the constant visibility of the `yml` format as private.

Because as @stof [mentionned](#28815 (comment)) `yml` isn't a format, so the constant  shoudn't be public.

Otherwise, this will be confusing while using autocomplete, you see two formats. The user can ask himself if there is a difference between `yaml` / `yml`.

No need of that :)

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Fixed tickets | -
| License       | MIT
| Doc PR        |

Commits
-------

674b359 'yml' is not a format and shouldn't be visible as constant from 'YamlEncoder' class
@xabbuh xabbuh added this to the 4.2 milestone Mar 8, 2019
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