Skip to content

[Yaml] Remove internal arguments from the api #21230

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 1 commit into from

Conversation

GuilhemN
Copy link
Contributor

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

This PR removes internal constructor arguments of the Parser class.
This should break nothing as they are only used to build errors message and are clearly meant for internal uses.

This would allow to have a nicer api for #21194 (comment).

@nicolas-grekas
Copy link
Member

Not sure this should be done in a separate PR: it belongs to #21194, and misses a BC layer

@xabbuh
Copy link
Member

xabbuh commented Jan 10, 2017

I think it's okay to handle it in a separate PR, but it indeed misses the BC layer and should be documented in the upgrade files.

@nicolas-grekas
Copy link
Member

A separate PR will create useless conflicts with #21194, if we happen to pass the array of tag resolvers I was talking about there, but as you want of course.

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Jan 10, 2017

As I would have to upgrade the bc layer in #21194, I'll do it directly in it. I opened this a bit too quickly 😐

@GuilhemN GuilhemN closed this Jan 10, 2017
@GuilhemN GuilhemN deleted the REMOVEINTERNALARGUMENTS branch January 10, 2017 18:09
@GuilhemN GuilhemN restored the REMOVEINTERNALARGUMENTS branch January 19, 2017 19:09
xabbuh added a commit that referenced this pull request Jan 21, 2017
This PR was squashed before being merged into the 3.3-dev branch (closes #21350).

Discussion
----------

[Yaml] Remove internal arguments from the api

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

Reopening of #21230 because of [@xabbuh's comment](#21194 (comment)).

> This PR removes internal constructor arguments of the `Parser` class.
> This should break nothing as they are only used to build errors message and are clearly meant for internal uses.
>
> This would allow to have a nicer api for #21194 (comment).

Commits
-------

ebae4ff [Yaml] Remove internal arguments from the api
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.

4 participants