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

[gardening] remove unecessary headers and plists causing SPM warnings #2203

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

AndrewSB
Copy link
Member

@AndrewSB AndrewSB commented Sep 5, 2021

@BasThomas i was seeing a warning after i switched from my fixes-ci branch to your fixed development

i think its safe to delete these, since they aren't needed (at least for SPM) to generate a library. not very confident about this breaking Carthage/Cocoapods though, so lets merge this after CI can validate that?

Copy link
Contributor

@BasThomas BasThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for the cleanup!

.package(url: "https://github.com/AliSoftware/OHHTTPStubs.git", .upToNextMajor(from: "9.0.0")) // dev
.package(url: "https://github.com/Alamofire/Alamofire.git", from: "5.4.3"),
.package(url: "https://github.com/ReactiveCocoa/ReactiveSwift.git", from: "6.6.1"),
.package(url: "https://github.com/ReactiveX/RxSwift.git", from: "6.2.0"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to specify the same major version as podspec than to use the fixed version.
Is there a reason why you chose a fixed version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the from: "semvar" is an alias for up to the next major version. so selecting Alamofire.git", from: "5.4.3" allows any version from 5.x.x

we should definitely match what the Podspec & Cartfile have, I wish there was one place we could define all of those minimum versions to stay consistent.

are the PodSpec/Cartfile significantly different?

Copy link
Member

@OhKanghoon OhKanghoon Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is from: "5.4.3" correct to include all 5.x.x? I didn't know that. 😅
It seems confusing to me
because the document saying that it starting with the minimum version going up to the next major version.

In the podspec file it is written as ~> 5.0
how about 5.0.0 in the Package.swift file? (from: "5.0.0" or .upToNextMajor(from: "5.0.0"))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh you're right, its technically 5.4.3 until 6.0.0, not including 6.0.0.

so 5.4.2 or below aren't acceptable

yeah, it is a little confusing, they're updating the SPM syntax very often

Copy link
Member

@OhKanghoon OhKanghoon Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, I think it would be good to change like podspec. (Alamofire, RxSwift etc..)
What do you think about it?

e.g) Alamofire

  • podspec : ~> 5.0
  • SPM: from: "5.0.0" or .upToNextMajor(from: "5.0.0")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BasThomas
Copy link
Contributor


❌  error: Build input file cannot be found: '/Users/distiller/Moya/Moya/Sources/Supporting Files/Info.plist' (in target 'MoyaTests' from project 'Moya')

:(

@AndrewSB
Copy link
Member Author

AndrewSB commented Sep 6, 2021

let me see if i can fix that build error

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