Skip to content
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

[Config] Use LoaderResolverInterface for type-hinting #2785

Closed
wants to merge 1 commit into from
Closed

[Config] Use LoaderResolverInterface for type-hinting #2785

wants to merge 1 commit into from

Conversation

jmikola
Copy link
Contributor

@jmikola jmikola commented Dec 5, 2011

Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -

I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.

fabpot added a commit that referenced this pull request Dec 5, 2011
Commits
-------

7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting
48b084e fixed typo
8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775)
240796e [Bridge] [Doctrine] fixed coding conventions.
7cfc392 check for session before trying to authentication details
648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768)
3976b7a [DoctrineBridge] fixed CS
9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765)
3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity.
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements

Discussion
----------

[Config] Use LoaderResolverInterface for type-hinting

```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```

I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
@lsmith77
Copy link
Contributor

lsmith77 commented Dec 5, 2011

hmm why is this PR not marked as closed if it was merged?

@stloyd
Copy link
Contributor

stloyd commented Dec 5, 2011

GH have some issues with autoclosing since last friday...

@fabpot fabpot closed this Dec 5, 2011
@lsmith77
Copy link
Contributor

lsmith77 commented Dec 5, 2011

@jmikola since this is a BC break it should get an entry in the upgrading guide.

and yes it does affect anyone that was implementing the interface:
FriendsOfSymfony/FOSRestBundle#155

@jmikola
Copy link
Contributor Author

jmikola commented Dec 5, 2011

That second merge doesn't look correct, either.

To @lsmith77's point, I'm fine if this change is better suited for the master branch, since it is a BC break. I didn't think it'd be a problem, since the methods I modified weren't labeled @api. Let me know if you'd like this rebased against master and submitted as a new PR, though.

fabpot added a commit that referenced this pull request Dec 7, 2011
Commits
-------

ea2b0e5 Note LoaderResolverInterface type hinting in the 2.1 changelog

Discussion
----------

Note LoaderResolverInterface type hinting in the 2.1 changelog

Although #2785 was originally opened against 2.0, its merge to master seems to be intentional. This documents the BC break that some experienced (see: FriendsOfSymfony/FOSRestBundle#155).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants