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

observer leak bug fix #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

observer leak bug fix #221

wants to merge 1 commit into from

Conversation

bestwnh
Copy link

@bestwnh bestwnh commented Sep 28, 2014

fix a observer leak if the developer set the show status to YES before
add the handler.

fix a observer leak if the developer set the show status to YES before
add the handler.
@bestwnh bestwnh mentioned this pull request Sep 28, 2014
@bestwnh
Copy link
Author

bestwnh commented Sep 28, 2014

I also create an issue about this bug. #222

@martsen
Copy link

martsen commented Jan 21, 2015

Thanks! It really helped me.

@dennisreimann
Copy link

Can this be brought in? It otherwise crashes a lot of times for me when moving away from a controller that has pull to refresh and/or infinite scrolling enabled...

@bestwnh
Copy link
Author

bestwnh commented Mar 8, 2015

@dennisreimann I don't understand. Could you provide more information or some sample code?

@dennisreimann
Copy link

@bestwnh All is fine, I just wanted to reassure that this is also happening for me in case your fix is not present. So please, merge this into the official version!

@bestwnh
Copy link
Author

bestwnh commented Mar 8, 2015

@dennisreimann I really want to merge and already pull a request long time ago(Oct of 2014). But I don't receive any response. And I contact @samvermette and here is his response:

Hey Galvin,

Indeed I've long decided to give up on some of my classes, including SVPullToRefresh. It's especially true for that class since I've never actually used it in a production app. 

There are over 570 forks of SVPullToRefresh out there, I suggest you find the best and most actively maintained and put your hopes in that one :)

Sam

@dennisreimann
Copy link

Well, that's sad.

@bestwnh
Copy link
Author

bestwnh commented Mar 8, 2015

Yes. But he is still making some other amazing projects. So nothing to blame.
But it could be better if he can transfer this project to someone to continue.

@LawrenceHan
Copy link

Created a pull request in https://github.com/LawrenceHan/SVPullToRefresh
I will merge it after discusses with you.

@bestwnh
Copy link
Author

bestwnh commented May 12, 2016

@LawrenceHan Do whatever you want 😃

@LawrenceHan
Copy link

@bestwnh 昨天花了几个小时初步整理了一下pull request,fork到我的repo里了。
但是在合并进repo之前,我想先确认一下这个pull request是做什么,解决了什么问题,做过测试嘛。以防止引发其他问题。

所以能先说明一下,你的这个pull request解决了什么问题?

@bestwnh
Copy link
Author

bestwnh commented May 13, 2016

Oh, 同胞哦。问题说明在这上面有连接 #222 ,测试基本试过,其他人也用着没反馈新问题,其实就是加了个预判,不会引入什么新问题,你看看issue那边的说明然后看看相关代码就明白的了。

@LawrenceHan
Copy link

@bestwnh 已经合并了。

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.

4 participants