search

RxSwiftCommunity / Action

Abstracts actions to be performed in RxSwift.
MIT License
875 stars 150 forks source link

Drop testing dependencies #101

Closed fpillet closed 7 years ago

fpillet commented 7 years ago

The move to Swift 4 works because I applied just the minimal fixes needed for Quick to run in the case of Action. But playing with Quick revealed that there are many more tasks to complete to bring it up to part with Swift 4.

I think it would be a good idea to drop the Quick/Nimble dependencies for testing, and only rely on RxSwift and RxCocoa which are the true one dependencies this frameworks depends on.

freak4pc commented 7 years ago

@fpillet Not sure I necessarily agree. You mean drop and change everything to use XCTest? Moya also uses Quick/Nimble and so does many other frameworks. It's a matter of preference, really nothing too critical beyond that. I don't think bringing the code up to Swift 4 would be too crazy.

fpillet commented 7 years ago

Well I did some minimal changes to Quick (see my PR there at gihub.com/Quick/Quick) but Quick itself doesn't pass its own tests. Given the size of Action and the fact that other RxSwiftCommunity don't rely on a third-party testing framework, I think that restricting dependencies is a good thing.

I see RxSwift & community projects as the root dependencies for others to base on, not the other way around :) So why you're correct in mentioning other frameworks, it doesn't mean that we should follow their lead in this respect, IMHO.

ashfurrow commented 7 years ago

Welllll, I can see both sides. Certainly it's good for us to minimize dependencies, but Quick/Nimble are development dependencies; our users probably don't know that we use them.

I'm personally in favour of keeping them, both because Quick/Nimble make testing easier, and using Quick/Nimble instead of XCTest is what I would consider a "best practice", and I'd like to promote their use. I'm pretty biased, though.

Anyone from @RxSwiftCommunity/contributors have any strong opinions on this?

sunshinejr commented 7 years ago

I'd say that instead of replacing the tests API to XCTest from Quick/Nimble, the energy could be invested in helping Quick/Nimble port to 4.0 faster. But I'm also biased, though.

freak4pc commented 7 years ago

My opinion is 👍 for keeping Quick/Nimble. It's been a strongly used dependency for many open source project and as @ashfurrow suggested, it is just a testing dependency so no IPA size issues there

our users probably don't know that we use them.

They probably shouldn't care even if they do, it is mainly used to make sure the library is passing its defined tests, so it shouldn't be of any matter to an external developer really

fpillet commented 7 years ago

The PR I pushed to Quick fixes just enough for Action's tests to seemingly pass on Swift 4. I know they are not complete enough for Quick to pass its own tests so I'm not sure it will get accepted. I have neither the time nor interest going further shaving the yak, although Action is a dependency I care very much about. Will wait for the Quick project to come up with a Swift 4-compatible version then!