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

ContainerAwareTrait #2

Merged
merged 4 commits into from
Mar 21, 2014
Merged

ContainerAwareTrait #2

merged 4 commits into from
Mar 21, 2014

Conversation

piotr-cz
Copy link
Contributor

@piotr-cz piotr-cz commented Mar 9, 2014

Please review.

Usage:

use Joomla\DI\ContainerAwareInterface,
    Joomla\DI\ContainerAwareTrait;

class WebApplication implements ContainerAwareInterface
{
    use ContainerAwareTrait;
}

Unfortunately one is not able to change $container access (to protected):

Fatal error: WebApplication and Joomla\DI\ContainerAwareTrait define the same property ($container) in the composition of WebApplication. However, the definition differs and is considered incompatible.

@piotr-cz piotr-cz mentioned this pull request Mar 9, 2014
@mbabker
Copy link
Contributor

mbabker commented Mar 9, 2014

Could you make this @since 1.2? Also, could you add a note to the README file with some info about the trait?

@piotr-cz
Copy link
Contributor Author

Done, I'd appreciate if you check out the readme addition.

@mbabker
Copy link
Contributor

mbabker commented Mar 10, 2014

Looks good to me

On Monday, March 10, 2014, Piotr notifications@github.com wrote:

Done, I'd appreciate if you check out the readme addition.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-37228327
.

mbabker added a commit that referenced this pull request Mar 21, 2014
@mbabker mbabker merged commit 4866c58 into joomla-framework:master Mar 21, 2014
@piotr-cz piotr-cz deleted the feature-ContainerAwareTrait branch March 21, 2014 14:15
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

2 participants