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

[Do not merge] Adapt Zinc to use the new LibraryManagement API #335

Closed
wants to merge 7 commits into from

Conversation

Duhemm
Copy link
Contributor

@Duhemm Duhemm commented Jul 6, 2017

@typesafe-tools
Copy link

dbuild has checked the following projects against Scala 2.12:

Project Reference Commit
sbt 1.0.x sbt/sbt@d464383
zinc pull/335/head bde9b92
io 1.0 sbt/io@21ed6ec
librarymanagement pull/125/head sbt/librarymanagement@9d1b8de
util 1.0 sbt/util@c76d262
website 1.0.x sbt/website@7e644bc

❌ The result is: FAILED
(restart)

Copy link
Member

@jvican jvican left a comment

Choose a reason for hiding this comment

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

Looking forward to this passing CI. I just have one question. The rest of changes LGTM!

@@ -10,7 +10,7 @@ object Dependencies {

private val ioVersion = "1.0.0-M12"
private val utilVersion = "1.0.0-M25"
private val lmVersion = "1.0.0-X16"
private val lmVersion = "1.0.0-SNAPSHOT"
Copy link
Member

Choose a reason for hiding this comment

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

Can we depend on a stable version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the plan once the new library management API is merged and released.

dependencies,
None,
ZincLMHelper.DefaultConfigurations)
libraryManagement.moduleDescriptor(wrapper, dependencies, None)
Copy link
Member

Choose a reason for hiding this comment

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

Component is no longer part of the configurations out of the box, so that might needs adjustment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

def addSbtLm(p: Project): Project = addSbtModule(p, sbtLmPath, "lm", libraryManagement)
def addSbtLmCore(p: Project): Project = addSbtModule(p, sbtLmPath, "lmCore", libraryManagementCore)
def addSbtLmIvy(p: Project): Project = addSbtModule(p, sbtLmPath, "lmIvy", libraryManagementIvy)
def addSbtLmIvyTest(p: Project): Project = addSbtModule(p, sbtLmPath, "lmIvy", libraryManagementIvy, Some(Test))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eed3si9n @dwijnand I changed that when I played with dbuild. Is this necessary / correct?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, thanks.

@Duhemm
Copy link
Contributor Author

Duhemm commented Jul 13, 2017

Should we rename the zincIvyIntegration module? zincLmIntegration?

@eed3si9n
Copy link
Member

@Duhemm

Should we rename the zincIvyIntegration module? zincLmIntegration?

Yes. Let's do it.

@Duhemm
Copy link
Contributor Author

Duhemm commented Jul 13, 2017

Done

@jvican
Copy link
Member

jvican commented Jul 14, 2017

When is the new LM api gonna be released? This is blocking adding a new method to the public API.

@eed3si9n
Copy link
Member

LM API has been merged. I am now starting to merge PRs for Zinc locally to make sure they compile.

@jvican
Copy link
Member

jvican commented Jul 15, 2017

Superseded by #366.

@jvican jvican closed this Jul 15, 2017
@Duhemm Duhemm mentioned this pull request Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants