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

Add SPM #97

Closed
wants to merge 1 commit into from
Closed

Add SPM #97

wants to merge 1 commit into from

Conversation

SMillerDev
Copy link
Contributor

Gonna build packages for SPM and make the iOS department pay for it.

Make PHP developers great again

@simdani
Copy link

simdani commented Sep 19, 2019

#98

@simdani
Copy link

simdani commented Sep 23, 2019

@SMillerDev your build is failing because of UIKit imports, will you fix it? :)

@SMillerDev
Copy link
Contributor Author

I might

@SMillerDev
Copy link
Contributor Author

You clearly have a higher interest in it then beating your colleagues though. You might be better off making your own pull request. You could be the hero that fixes this for all users of Marky-Mark.

@lvnkmn
Copy link
Collaborator

lvnkmn commented Sep 24, 2019

Couldn't (really) wait for this to be merged, it turned out to work just fine for me. Nice one Sean!

@SMillerDev
Copy link
Contributor Author

@lvnkmn are you saying this implementation is correct and my tests are just wrong?

@lvnkmn
Copy link
Collaborator

lvnkmn commented Sep 25, 2019

Not trying to offend your tests here. It works for what I need it to do, that's all.

@SMillerDev
Copy link
Contributor Author

I'm not at all offended, just disappointed in my google skills 😄

@simdani
Copy link

simdani commented Sep 25, 2019

It works well if you import UIKit in missing files :)

@SMillerDev
Copy link
Contributor Author

Remaining failure seems to be an issue with iOS 13 more then SPM. @M2Mobi/ios any ideas?

@lvnkmn lvnkmn mentioned this pull request Jan 21, 2020
@lvnkmn
Copy link
Collaborator

lvnkmn commented Jan 21, 2020

I have been pointing to @SMillerDev 's repo because of this for quite a bit now. Any news on when this can be merged (and if needed completed)?

@lvnkmn lvnkmn self-requested a review January 29, 2020 12:30
@lvnkmn
Copy link
Collaborator

lvnkmn commented Jan 29, 2020

@Basca, @ThijsBouma This branch and PR works for me in swift package manager, however there's a couple of code changes that I don't think are appropriate:

  1. Changes in podspec should not be needed for this
  2. Commented out code in package.swift

Aside from that, some build settings have been changed, but have not resulted in travis tests actually succeeding.

What I'll do:

  • I'll create a new pull request, using the changes that make sense.
  • I'll close this PR as soon as that one is up.

What I'd like to know from you:
To what extend are you relying on the travis checks passing. Is local build and passing tests good enough? if that's the case we might be able to release this soon.

@SMillerDev
Copy link
Contributor Author

Aside from that, some build settings have been changed, but have not resulted in travis tests actually succeeding.

Afaik this was an iOS 13/Xcode 11 issue. If you can make a pull request to fix travis with Xcode 11 I can clean up this review a bit.

@lvnkmn
Copy link
Collaborator

lvnkmn commented Jan 29, 2020

Closed in favor of #101

@lvnkmn lvnkmn closed this Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants