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

Adds linkable library for all dependencies generated in a Marathon scripts #153

Merged
merged 3 commits into from Dec 10, 2017

Conversation

3 participants
@ashfurrow
Copy link
Contributor

ashfurrow commented Dec 9, 2017

This PR adds a a linkable library for the dependencies generated in Marathon scripts. tl;dr Danger uses Marathon to download its dependencies, but those deps need to be exposed to be linked against by Danger.

Paired on this with @orta. We used the name Dependencies for everything since it's pretty generic and captured all of the root-level dependencies. This will allow us to change this line:

https://github.com/danger/danger-swift/blob/5fd83392054054740cb4922a44fd4d59d2d6b05c/Sources/Runner/Commands/Runner.swift#L61

... to include libArgs += ["-lDependencies"] and expose Danger-Swift plugin symbols to Dangerfile.swift files. Let me know if I can clarify anything – thanks!

ashfurrow added some commits Dec 9, 2017

@ashfurrow

This comment has been minimized.

Copy link
Contributor

ashfurrow commented Dec 9, 2017

After removing a cached version of the generated Package.swift, we were able to verify that this works as-epected locally.

@@ -364,6 +364,7 @@ public final class PackageManager {
"import PackageDescription\n\n" +
"let package = Package(\n" +
" name: \"\(masterPackageName)\",\n" +
" products: [.library(name: \"Dependencies\", type: .dynamic, targets: [\"\(masterPackageName)\"])],\n" +

This comment has been minimized.

@JohnSundell

JohnSundell Dec 9, 2017

Owner

What do you think about calling this MarathonDependencies to be a bit more clear where it comes from?

This comment has been minimized.

@ashfurrow

ashfurrow Dec 9, 2017

Contributor

Good idea!

@JohnSundell
Copy link
Owner

JohnSundell left a comment

Beautiful ❤️

@orta

This comment has been minimized.

Copy link
Collaborator

orta commented Dec 10, 2017

Yep - this is cool with me too. Let's do it.

@orta orta merged commit 10d4ae9 into JohnSundell:master Dec 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@orta

This comment has been minimized.

Copy link
Collaborator

orta commented Dec 10, 2017

@ashfurrow

This comment has been minimized.

Copy link
Contributor

ashfurrow commented Dec 10, 2017

🎉 I'll send a PR to Danger-Swift to use this.

@ashfurrow ashfurrow deleted the ashfurrow:patch-1 branch Dec 10, 2017

@JohnSundell

This comment has been minimized.

Copy link
Owner

JohnSundell commented Dec 10, 2017

Awesome 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment