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

Fix SPM paths #105

Merged
merged 3 commits into from Feb 26, 2018
Merged

Conversation

kdawgwilk
Copy link
Contributor

Trying to depend on RxAlamofire was failing because paths are funky this specifies the right directory so that other packages can depend on this library

@freak4pc
Copy link
Member

Hey @kdawgwilk - Many thanks for this fix!

One blocker here is that, even thought swift build works fine, swift test does not (since its missing dependencies).

Adding the missing dependencies is not a big issue - The big issue here is OHTTTPStubs, that in itself doesn't have a Package.swift file so can't be depended on by a SPM Package.

If you have a thought on how to go around this, let me know - otherwise I'm not sure this can happen at the moment :)

Let me know
Shai

@orta
Copy link
Member

orta commented Feb 25, 2018

The Moya crew have been building a stubs engine with spm support FWIW: https://github.com/Moya/Harvey

@freak4pc
Copy link
Member

Yup, that's definitely another good path to go at - even though SPM support alone wouldn't be a good reason to swap out a stubbing library. I've been meaning to check Harvey but didn't get the chance yet :)

Definitely something to look into, but this ticket (as of now) probably can't happen IMO.

@kdawgwilk
Copy link
Contributor Author

IMO this should not be blocked by the fact that you can’t run tests using SPM because before this PR they weren’t running using SPM anyways. This adds new support without breaking anything that wasn’t already broken

@freak4pc
Copy link
Member

@kdawgwilk That makes sense to me as well (for the time being).
The fact we can't test doesn't mean we should block support for everyone, so for now I've removed the test target specifically from Package.swift

Thanks !

@freak4pc freak4pc merged commit e8e058b into RxSwiftCommunity:master Feb 26, 2018
@rxswiftcommunity
Copy link

Thanks a lot for contributing @kdawgwilk! I've invited you to join the
RxSwiftCommunity GitHub organization – no pressure to accept! If you'd like
more information on what this means, check out our contributor guidelines
and feel free to reach out with any questions.

Generated by 🚫 dangerJS

@freak4pc
Copy link
Member

Thanks @kdawgwilk, and welcome to RxSwiftCommunity!

@kdawgwilk
Copy link
Contributor Author

kdawgwilk commented Feb 26, 2018

Alright thanks! How soon can a release tag be expected to include this fix? Right now we can point SPM to the master branch but that means any dependency we use has to also be using a branch instead of a tagged version. SPM doesn't like it when tagged release version of a dev uses a branch for one of its own dependnecies

@freak4pc
Copy link
Member

I'll try to take care of this a bit later :) We need to fix something related to deployment before adding a new release tag. Will update as it happens.

@freak4pc
Copy link
Member

@kdawgwilk 4.1.0 was just pushed. Thanks for your assistance !

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.

None yet

3 participants