Skip to content

Conversation

@Atry
Copy link
Contributor

@Atry Atry commented Jun 1, 2016

Because ReleasePlugin requires settings like managedClasspath defined in JvmPlugin

Because ReleasePlugin requires settings like managedClasspath defined in JvmPlugin
@Atry
Copy link
Contributor Author

Atry commented Jun 8, 2016

Ping @jroper

@jroper
Copy link
Member

jroper commented Jul 18, 2016

I think it would be better to make those dependencies optional, ie depend on managedClasspath only if it's set. I don't think release plugin should depend on JvmPlugin, it's conceivable that you may release an sbt project that doesn't use the JVM.

@Atry
Copy link
Contributor Author

Atry commented Jul 18, 2016

In order to make these dependencies optional and still keep the same behavior of the current sbt-release, we need the following steps:

  1. Create a ReleaseCorePlugin, which does not depend on JvmPlugin.
  2. Let ReleasePlugin depend on both ReleaseCorePlugin and JvmPlugin.
  3. Move those settings that does not require managedClasspath from ReleasePlugin to ReleaseCorePlugin.

@Atry
Copy link
Contributor Author

Atry commented Aug 12, 2016

Hi, @jroper ,

Since this PR is a bug fix, could you merge this PR for now?

Without this PR, if some one added sbt-release dependency in /project/plugins.sbt for a multi-project build and disabled JvmPlugin in /non-jvm-subproject/build.sbt for one of these projects, the build would be broken.

@jroper
Copy link
Member

jroper commented Aug 14, 2016

I'll merge if you update it to make the dependency on managedClasspath optional. You don't need to do the multiple plugins that you suggested, just use the ? operator, eg:

val maybeClasspath: Option[Classpath] = managedClasspath.?.value

Requiring the JvmPlugin to be enabled in order for the ReleasePlugin to work is unnecessary.

Also, I'd like to see a scripted test added to ensure that the bug is fixed and doesn't regress in future.

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.

2 participants