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

remove ->getContainer() call inside internal App class inside setUpEventWithRequestResponse() method #146

Merged
merged 1 commit into from
Dec 6, 2015

Conversation

samsonasik
Copy link
Member

No description provided.

@gianarb
Copy link
Contributor

gianarb commented Nov 29, 2015

I am not interested to remove this method..
Why would you remove it?

@samsonasik
Copy link
Member Author

it is called in internal method, like on getEventManager(), we just use $this->container->get('event_manager');, if we call outside, we still can call $app->getContainer()

@samsonasik
Copy link
Member Author

I don't remove the getContainer() method, I just remove its call, ->getContainer() inside App class, if we need call outside, we still can call $app->getContainer()

@samsonasik
Copy link
Member Author

@gianarb I think the better PR title is "remove ->getContainer() call inside internal App class inside setUpEventWithRequestResponse() method" I will update PR title

@samsonasik samsonasik changed the title remove ->getContainer() call inside internal App class remove ->getContainer() call inside internal App class inside setUpEventWithRequestResponse() method Nov 29, 2015
@gianarb
Copy link
Contributor

gianarb commented Nov 29, 2015

At the moment I don't see ways to remove this method.. Maybe we can remove it in the future ;)

@samsonasik
Copy link
Member Author

why? this is called in inside setUpEventWithRequestResponse(), if we need call after instantiation, we still can call $app->getContainer(), but not inside App class, also, for consistency in compare with container call inside getEventManager() method.

@samsonasik
Copy link
Member Author

again, I'm not remove getContainer() method, I just removed its call inside App class method, and for consistency with getEventManager() which is used $this->container instead.

@gianarb
Copy link
Contributor

gianarb commented Dec 6, 2015

Thanks

gianarb pushed a commit that referenced this pull request Dec 6, 2015
remove ->getContainer() call inside internal App class inside setUpEventWithRequestResponse() method
@gianarb gianarb merged commit b9c15e6 into pennyphp:master Dec 6, 2015
@samsonasik samsonasik deleted the remove-getcontainer-call branch December 7, 2015 01:02
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.

2 participants