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

Allow multiple aliases per bean and add return type aliases #99

Merged
merged 4 commits into from
Jun 12, 2017

Conversation

codeliner
Copy link
Contributor

Resolves #74

Implementation is very close to what we've discussed in the issue, except that I use a single @Alias for named and for return type aliases. It makes the implementation simpler ("aliases" attribute of bean can be a collection).

Like noted in the issue: the PR introduces a BC break

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 90.174% when pulling ac6ee89 on codeliner:feature/advanced_aliases into fefe256 on bitExpert:master.

@shochdoerfer
Copy link
Member

PR looks good. 2 minor issues:

  • What is the reason of not supporting native types as aliases? Sure, logically it does not make much sense, but is there a need for for this restriction? If so, it might be better to move the check to the Alias annotation class, that way we can be sure we deal with valid Aliases all the time.
  • The exception which is thrown when a duplicate alias exists should also contain the method name. IIRC this is stored in $aliases[$alias]. That should help when it comes to debugging.

@shochdoerfer shochdoerfer added this to the 0.9.0 milestone Jun 11, 2017
@codeliner
Copy link
Contributor Author

@shochdoerfer Reason was that it does not make much sense ;) But you're right user can take care of it. And if a user wants to use "array" as service id (why ever ...) we can allow it. Simplifies the code 👍

Also good hint with including the method name. Added it to exception message.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 90.064% when pulling 251d9a0 on codeliner:feature/advanced_aliases into fefe256 on bitExpert:master.

@shochdoerfer shochdoerfer merged commit 5baa3e1 into bitExpert:master Jun 12, 2017
@shochdoerfer
Copy link
Member

...and merged. Thanks @codeliner for your contribution. Will release the next version soonish but first want to refactor the parameter handling as well.

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.

[Feature ]Automatically generate alias from return type declaration
3 participants