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

[5.7] [WIP] bug fix for app()->call() #26915

Closed
wants to merge 5 commits into from

Conversation

imanghafoori1
Copy link
Contributor

@imanghafoori1 imanghafoori1 commented Dec 19, 2018

Some changes in the BoundMethod class to make a fix addressing these issues:

#26851
#22687
#26837

Following the my previous bug fix #26852 which caused a new nasty bug and eventually got reverted...
I created an other PR to add the missing tests to cover more cases #26897

After one sleepless night I am coming up with an other fundamental solution for making it really fixed.

I know I am asked not to make a PR again for that feature but please let me try again.
This time much more vigilant, with more tests.

I will add more tests tomorrow to be really sure that no more revert will happen

@deleugpn
Copy link
Contributor

You have a better chance by targeting the master branch and fixing the issue for the next release. It gives more time for thorough test.

@taylorotwell
Copy link
Member

Absolutely no way this gets merged on 5.7.

@imanghafoori1
Copy link
Contributor Author

@taylorotwell
Ok I just continue working on it.
You may consider merging it for 5.8 or ignore it anyway.

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.

3 participants