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

Dispatcher handlercache #13854

Merged
merged 3 commits into from
Feb 24, 2019
Merged

Dispatcher handlercache #13854

merged 3 commits into from
Feb 24, 2019

Conversation

dschissler
Copy link
Contributor

@dschissler dschissler commented Feb 23, 2019

…a handler hash cache.

Hello! This is another proof of concept PR

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I did not write tests for this because a large refactor of the Dispatcher is needed. This is only to remove one small wart. Creating tests at this time would be premature.

This PR is attempting to solve the use of DI::wasFreshInstance in Phalcon\Dispatcher. I think that DI::wasFreshInstance should be completely removed in 4.0.x since it is only used in this one place for this hack. I'm attempting to see if a hash hit map of all handlers seen could be used instead of the use of wasFreshInstance.

I'm attempting to understand everything that would need to be changed to get the Dispatcher to run initialize first every time as first began by @virgofx but wow is it some involved code. I'm not sure that I'm setup to change something so large at this point. So I'm starting small. This PR may never be merged or I could also grow it to be more encompassing.

@dschissler dschissler closed this Feb 23, 2019
@dschissler dschissler reopened this Feb 23, 2019
@dschissler

This comment was marked as abuse.

@niden niden added the enhancement Enhancement to the framework label Feb 24, 2019
@niden
Copy link
Member

niden commented Feb 24, 2019

Approving/merging this. Next time though we need tests to be 100% sure that nothing was broken.

Thank you @dschissler

@niden niden merged commit 187a2a4 into phalcon:4.0.x Feb 24, 2019
@dschissler

This comment was marked as abuse.

@sergeyklay
Copy link
Contributor

@dschissler I think you can just amend the description of the pull request with something like that "I did not provide tests for the following reasons ...". This is ok :) Anyway thank you for the contributting, and for helping us make Phalcon better.

@dschissler

This comment was marked as abuse.

@dschissler dschissler deleted the dispatcher_handlercache branch February 24, 2019 19:37
@niden niden added the documentation Documentation required label Apr 9, 2019
@niden niden added 4.0 and removed documentation Documentation required labels Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to the framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants