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

RxLifeCycle #32

Open
onmyway133 opened this issue Nov 17, 2017 · 2 comments
Open

RxLifeCycle #32

onmyway133 opened this issue Nov 17, 2017 · 2 comments

Comments

@onmyway133
Copy link

onmyway133 commented Nov 17, 2017

I've read https://github.com/RxSwiftCommunity/contributors and I thought this https://github.com/onmyway133/RxLifeCycle should belong to RxSwiftCommunity. I've used this approach in many of projects, and now it should be a pod, in hope that people find it useful ❤️

The only thing left is to test view controller containment, which I still don't know how to do, here is my question on twitter https://twitter.com/onmyway133/status/931448335625588736, if you know how to test it, please help 🙏

Basically, I would like to avoid swizzling at all cost, and instead use composition, so you can just declare an instance and use it.

Thanks, hope to hear your feedback

@freak4pc
Copy link
Member

freak4pc commented Nov 19, 2017

This comment is entirely personal opinion, so don't take it as a "set in stone" thing.
I want to start off by thanking you on the amount of work you spent on the project, as well as the time you spent submitting it to RxSwiftCommunity. It is definitely appreciated ! 😄

First, I think the swizzled approach isn't "nice" - but it is working well since it has been battle tested with various code bases as well as the fact some of the DelegateProxies are using those same methods.

So - generally I'm against swizzling, but in such a battle tested way I live peacefully with it :)

Second, I am usually against having two libraries in RxSwiftCommunity that do the exact same thing unless one of them has a substantial benefit over the other, or the other one has been heavily under-maintained. This doesn't seem to be the case here, and @devxoul's RxViewController seems to be a popular, useful and maintained choice.

Lastly, I'm not a huge fan of the API you describe here. Having a wrapper object just to get Reactive Observation seems a bit excessive to me. It also breaks the "discovering" syntax of RxCocoa (e.g. item.rx.propety) by adding a custom .rxLifeCycle namespace.

Overall, my personal opinion is we don't need another life cycle project in RxSwiftCommunity, but I'm 100% open to any opinions on this and I'm definitely not religious to it.

@bobgodwinx
Copy link
Member

@onmyway133 Can you please checkout this repo https://github.com/devxoul/RxViewController it looks like what you're doing is already available and well tested. You can also add your contribution to it and I would ask @devxoul to bring in all his @RxSwift related repos into the @RxSwiftCommunity in order to avoid further confusion.

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

No branches or pull requests

4 participants
@freak4pc @bobgodwinx @onmyway133 and others