Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

fixed seg fault in redis adapter on PHP 7 #53

Merged
merged 1 commit into from
Dec 22, 2015
Merged

fixed seg fault in redis adapter on PHP 7 #53

merged 1 commit into from
Dec 22, 2015

Conversation

marc-mabe
Copy link
Member

After changing selected database on an already connected object
the RedisResourceManager was again trying to connect which crashed.

After changing selected database on an already connected object
the RedisResourceManager was again trying to connect which crashed.
@marc-mabe marc-mabe added the bug label Dec 21, 2015
@marc-mabe marc-mabe self-assigned this Dec 21, 2015
@marc-mabe
Copy link
Member Author

To test added as the current tests reflected this seg fault already on PHP 7

@marc-mabe marc-mabe merged commit ef4867d into zendframework:master Dec 22, 2015
marc-mabe added a commit that referenced this pull request Dec 22, 2015
fixed seg fault in redis adapter on PHP 7
marc-mabe added a commit that referenced this pull request Dec 22, 2015
marc-mabe added a commit that referenced this pull request Dec 22, 2015
@marc-mabe marc-mabe deleted the redis_php7_segfault branch December 22, 2015 10:27
@gianarb
Copy link
Contributor

gianarb commented Dec 22, 2015

Sorry little question,

is a good practice self merge own PRs without any review?

For me this PR is perfect :) But I don't know if its the correct flow.

@marc-mabe
Copy link
Member Author

hi @gianarb, normally you are right! I did it only this time because it's a simple fix and it breaks very hard. It also breaks nearly all tests running on PHP 7 where it makes it very hard to find out if other changes are correct.

Please tell me if I should avoid this practice in the future and/or what I can do if it doesn't get reviewed for a long time (which is sometimes the case here as this component requires a lot of setup to run).

@gianarb
Copy link
Contributor

gianarb commented Dec 22, 2015

No problem for me :)
Thanks for your work!

@marc-mabe marc-mabe added this to the 2.6.0 milestone Feb 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants