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

Wip/autoplugins #374

Merged
merged 1 commit into from
Oct 22, 2014
Merged

Wip/autoplugins #374

merged 1 commit into from
Oct 22, 2014

Conversation

muuki88
Copy link
Contributor

@muuki88 muuki88 commented Oct 8, 2014

This is a work-in-progress pull-request. Feel free to add suggestions, corrections and ideas for the autoplugins.

AutoPlugin Dependency Hierarchy

SbtNativePackager
    UniversalPlugin
        WindowsPlugin
        DockerPlugin
        LinuxPlugin
            RpmPlugin
            DebianPlugin
                DebianNativePackaging
                JDebPackaging
    JavaAppPackaging
        AkkaAppPackaging
        JavaServerAppPackaging

Keys

For backwards compatibility there are all Keys inside packager.Keys or in the reference NativePackagerKeys.

All keys are defined in traits. The autoImport objects of the respecting plugin extends these traits
and define addition configurations if necessary. The structure should always look like this

object DockerPlugin extends AutoPlugin {

  object autoImport extends DockerKeys {
    val Docker = config("docker") extend Universal
  }

}

Archetypes

I included the archetype declarations. Mostly because I'm not sure how to enable plugins in a scala (multi) build file.

@jsuereth
Copy link
Member

jsuereth commented Oct 8, 2014

I'll be reviewing shortly (once I'm through the morning meetings). SUPER excited to see this, great work!

val Rpm = rpm.RpmPlugin.Rpm
val Windows = windows.WindowsPlugin.Windows

val autoImport = packager.Keys
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this was a big question in my head when I initially tried to do this when 0.13.5 was released. Should we split the keys into different autoImports per-plugin or just have one glob? Practically it probably doesn't matter, but you could see the sbt-native-packager was designed to have them be separate objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

My vote = different autoImports per plugin.

@huntc
Copy link
Contributor

huntc commented Oct 9, 2014

Great to see this happening @muuki88 !

My initial reaction was around AkkaAppPackaging. It'd be useful to have this separated out into its own project eventually as it doesn't feel core to the native packager; perhaps it should be a part of the Akka microservice project itself. @rkuhn ?

import sbt._
import java.io.File

object StageHelper {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be possible to improve the name here. Perhaps Stager even?

Note: I really dislike using the terms "helper", "util", "default" etc. - they become dumping grounds.

@huntc
Copy link
Contributor

huntc commented Oct 9, 2014

When this PR nears readiness I'll create an AutoPlugin that extends native packager with another format we're using internally. That should give these changes a good workout.

@huntc
Copy link
Contributor

huntc commented Oct 9, 2014

While we're doing a bit of re-factoring API wise, can we please have it so that stage is redeclared as:

val stage = TaskKey[File]...

I need a result from stage in the same way that dist returns a file. The file should represent the target folder of where the stage occurred.

@muuki88
Copy link
Contributor Author

muuki88 commented Oct 9, 2014

I will add these changes.
@jsuereth I like to branch a 0.8.x branch and use master as 1.0.0.
So we can release a 1.0.0-M1 with autoplugins.

@jsuereth
Copy link
Member

jsuereth commented Oct 9, 2014

Branching sounds likr a good plan

@rkuhn
Copy link

rkuhn commented Oct 9, 2014

@huntc Hmm, I’m not fluent enough in this problem space right now, can we discuss that next week?

@muuki88 muuki88 added this to the 1.0.0 Autoplugins milestone Oct 11, 2014
@muuki88
Copy link
Contributor Author

muuki88 commented Oct 11, 2014

I have change some of the stuff requested and fixed all the universal tests. The structure is slowly finalizing.

The EvilNotDependency hack works, if you define working as "it throws an exception if you enable JDebPackaging but don't disable DebianNativePackaging"

*
* @example Enable the plugin in the `build.sbt`
* {{{
* disablePlugins(DebianNativePackaging)
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 this needs to be changed now.

@jsuereth
Copy link
Member

I think this definitely LGTM, once travis responds.

@muuki88
Copy link
Contributor Author

muuki88 commented Oct 22, 2014

Removed the comments and tested locally (for the docker stuff).

muuki88 added a commit that referenced this pull request Oct 22, 2014
@muuki88 muuki88 merged commit 3de9f83 into master Oct 22, 2014
@muuki88 muuki88 deleted the wip/autoplugins branch October 22, 2014 16:09
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.

4 participants