-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
Enable comfortable Swift Package development #414
Conversation
Well, that's all good news to me- if you want to try fix CI, I bet it's updating an Xcode version in the travis yml in a few places ( I'm years out of the loop of the native ecosystem) otherwise me or @f-meloni will give it a shot Thanks! |
I'll have a look into the CI later today or this weekend. Btw, any objections regarding GitHub Actions? |
Big preference to using GH Actions instead, but I didn't want to force you to do it :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all thank you very much for taking the time to open this PR, in general the repo had some point that are now out of date and needed to be adressed, so thank you.
There are two main points that I want to hightlight.
We don't use Marathon anymore, as you correctly pointed out, but we have our own dependencies resolver https://github.com/danger/swift/tree/master/Sources/DangerDependenciesResolver (that is like Marathon with some additional capabilities)
So while the title was indeed wrong, and the link to Marathon is not needed, who uses Danger without using a Package.swift (mainly who uses it installing it from brew), might benefit from the content on the section.
For swift 5.3 I have one concern, that is that who has a Package supporting an older version of Swift or is using an older Xcode will have some issues on using Danger.
Given the latest swift version release is 5.3.3 a lot of people might be affected by this.
I'm totally fine with using GitHub Actions.
@orta Thoughts?
80c259e
to
3141aa8
Compare
Thanks for your feedback! I played a bit with the danger-swift code and now I'm close to understand what is actually going on with the dependency management. As far as I understand, the dependency resolver is needed to identify plugins right before SwiftPM tries to do the actual dependency management, right? That makes totally sense, even though the mysteriously changing
I do not really get the thoughts behind this sentence. Do you mean they benefit because the descriptions is close to what the
I'm not sure why someone might want to support an older version of Swift but wants to be up-to-date with tools like danger-swift. In case of an older Swift version, there is still the chance to use an older danger-swift version as well. But to be honest, the dependency management needs a revisit. This is clear to me every time I'm looking at the following screenshot: Speaking about dependencies and SwiftPM versions, your own dependencies like So what do you think? |
Yes, the The
I would like to offer a little bit of flexibility like one - two versions, and not always force everyone to upgrate to the latest xcode to get the latest Danger, there might be a fix they need or something similar. There is also to consider that many of the open source projects are still supporting from swift 5.1/5.2 and those might want to have the latest Danger in their Package.swift too. We ditribute Danger via brew, so this upgrade might also be annoying for who gets the new version, I know they can point an older formula, but I would like to minimise the work people has to do to keep danger working in the CI, with keeping at least swift 5.2 support, and maybe dropping it a little bit after swift 5.4 is out.
We decided to use SPM to handle all the tools we needed for development (but are removed to avoid everyone else to have to download them too), so the dependencies you have there are to lint, format, document the code etc. I'm open to make some improvements like using swift-log though, even if it doesn't actually improve the dependency graph (is even bigger than Logger), is nice to use something that Apple maintains for us, and is a lot more likely that that is already a dependency of someone's Package.swift, avoiding SPM to have to download two logging tools. |
Aye, I don't have much to add which hasn't been said already but just adding I think our dependency tree is a reasonable size - it's when dev deps are included in other people's apps that things become unreasonable |
Hey @lunij, now that we managed to get macOS 11 in our CI, I think we can get this PR in, can you please merge master back in the PR to see if there are other dependencies that can be updated and if it still passes CI? Thank you :) |
With the versions
5.2
and5.3
of the Swift Package Manager things have changed a lot. Resources are supported now and dev dependencies can also be described in thePackage.swift
file without the need of third-party packages. We just need to make sure the dev dependencies are not referenced directly by the main product. (see here) Also the generation of an Xcode project beforehand is not necessary anymore becauseopen Package.swift
will take care of both, the generation process and opening of the Xcode project.Thanks to all contributors of Marathon! 🙇
(deprecated since mid 2019)