App: finalise list of default plugins

[dilyand]

Here is a list of plugins currently in use in different Snowplow apps, to serve as a starting off point. Any suggestions about what we should remove / add?

"com.eed3si9n" % "sbt-assembly" % "xx"
"com.eed3si9n" % "sbt-buildinfo" % "xx"
"com.typesafe.sbt" % "sbt-native-packager" % "xx"
"org.scalastyle" %% "scalastyle-sbt-plugin" % "xx"
"io.get-coursier" % "sbt-coursier" % "xx"
"com.lucidchart" % "sbt-scalafmt-coursier" % "xx"
"com.lucidchart" % "sbt-scalafmt" % "xx"
"com.localytics" % "sbt-dynamodb" % "xx"
"io.github.davidgregory084" % "sbt-tpolecat" % "xx"
"org.scoverage" % "sbt-scoverage" % "xx"
"org.foundweekends" % "sbt-bintray" % "xx"

sbt-buildinfo or "scalify" settings?

[chuwy]

• "com.localytics" % "sbt-dynamodb" % "xx" - this one is nice to have, but obviously when necessary
• "io.get-coursier" % "sbt-coursier" % "xx" - this one can be excluded. Coursier will be default in SBT 1.3.0
• "io.github.davidgregory084" % "sbt-tpolecat" % "xx" - I'd exclude this one as well. It's too easy to add and make the build more opaque

[dilyand]

We should recommend "com.localytics" % "sbt-dynamodb" % "xx" for all projects that use DynamoDB as the preferred tool for tests (over mocks).

[dilyand]

@aldemirenes @benjben @chuwy @colmsnowplow @lukeindykiewicz @oguzhanunlu There are a number of decisions we need to make before we can close this issue; for example the one in the comment directly above. I'll create a new comment for each decision and then we can vote yay or nay with the +1 or -1 reaction.

[dilyand]

We should not use "com.eed3si9n" % "sbt-buildinfo" % "xx" and prefer rather "scalified" settings. For example:

lazy val scalifySettings = Seq(
    sourceGenerators in Compile += Def.task {
      val file = (sourceManaged in Compile).value / "settings.scala"
      IO.write(file, """package com.snowplowanalytics.example.generated
                       |object ProjectSettings {
                       |  val version = "%s"
                       |  val name = "%s"
                       |  val description = "%s"
                       |}
                       |""".stripMargin.format(version.value, name.value, description.value))
      Seq(file)
    }.taskValue

[dilyand]

We should not use "io.get-coursier" % "sbt-coursier" % "xx" and instead update the templates to use sbt 1.3.0 where coursier is the default.

[dilyand]

We should use "com.typesafe.sbt" % "sbt-native-packager" % "xx" rather than "org.foundweekends" % "sbt-bintray" % "xx".

[dilyand]

We should not include "io.github.davidgregory084" % "sbt-tpolecat" % "xx" in the default plugins.

[dilyand]

"com.timushev.sbt" % "sbt-updates" % "xx" should be a recommended plugin.

[lukeindykiewicz]

Let's use scalafmt. Both Idea and metals does use it, so there is no need for:

"org.scalastyle" %% "scalastyle-sbt-plugin" % "xx"
"com.lucidchart" % "sbt-scalafmt-coursier" % "xx"
"com.lucidchart" % "sbt-scalafmt" % "xx"

[lukeindykiewicz]

Revolver is also very nice way or running application via sbt, it forks the jvm, so there is no need to ctrl-c and killing by accident the sbt. link: https://github.com/spray/sbt-revolver It also supports ~, so it can test/run things continually

addSbtPlugin("io.spray" % "sbt-revolver" % "0.9.1")

[chuwy]

Just my two cents. I'm strongly against any plugin that is not required for reproducible build and can be added to a global sbt config. There are plenty of awesome plugins out there that make engineer's life easier and i'm all for using them, but including them into all projects is a whole different story. We'd like to keep the dependency footprint as small as possible, otherwise we introduce potential blockers for next Scala versions, introduce potential binary incompatibilities (through diamon dependencies) in a build, increase build time.

E.g:

• "com.localytics" % "sbt-dynamodb" % "xx" - required for DyanamoDB tests, :+1:
• "io.get-coursier" % "sbt-coursier" % "xx" - just makes life easier, can be added globally, does not (or should not) affect the build, 👎
• "com.typesafe.sbt" % "sbt-native-packager" % "xx" - required to produce an asset, CI/CD depends on it in projects configs, 👍
• "org.foundweekends" % "sbt-bintray" % "xx" - I'm not sure how it is a counterpart of sbt-native-packager, IMO its a whole different pluging that is also required if we continue to use bintray, 👍
• "com.timushev.sbt" % "sbt-updates" % "xx" - super-cool, but does not affect a build, can be used globally, 👎 etc.

[dilyand]

Thanks everyone for pitching in! If I can summarise the discussion so far:

There are many great plugins for sbt that we can be using and we want to retain freedom to pick our favourite tools. However, many of these are just about improving the developer's workflow. They're not strictly speaking required for a project.

We want to keep the templates as lightweight as possible, for the following reasons:

• easier to maintain
• provides more flexibility
• less risk for binary incompatibilities between builds
• less risk that a plugin we come to depend on might block upgrades (eg, to a newer version of Scala).

It makes sense then to only include in the templates those plugins that projects will depend on. Separately, we can have a list of useful plugins for sbt global settings in: https://github.com/snowplow-incubator/engineering-resources.

It looks like in general we agree on this list for the app template:

"com.eed3si9n" % "sbt-assembly" % "xx"
"org.scoverage" % "sbt-scoverage" % "xx"
"com.localytics" % "sbt-dynamodb" % "xx" // For DynamoDB tests

There is still some disagreement about "io.github.davidgregory084" % "sbt-tpolecat" % "xx". I've gone back and forth on this personally, but ultimately I think this should be in the list. If it's not, we might have to worry about all those scalacOptions separately. Surely, it's better to rely on an already thought-out and tested approach?

Finally, there's an outstanding question about whether "com.typesafe.sbt" % "sbt-native-packager" % "xx" and "org.foundweekends" % "sbt-bintray" % "xx" do the same thing. Strictly speaking they don't. However, if we think we would continue to use both Bintray and Docker, it might be more consistent to have the same approach to creating and publishing artefacts on both. Is it consistent to create Docker images with native packager and publish them with Travis; but to have a dedicated publishing plugin for Bintray? Maybe we should use native packager to create the artefacts and then Travis (release manager) can take care of publishing them to Bintray?

[lukeindykiewicz]

What about code formatter? The rules are something that we can talk about of course, but once we'll have some common config it will be a lot easier to work on the code that is autoformatted. There will be almost no doubt and no noise on reviews about code format. The downside is there will be bigger diffs in some cases because of formatting (ignore white spaces should help at least a little). Scalafmt does a pretty decent job and the project is active I think, so I would recommend this one.

[dilyand]

Oh, of course, I missed it in the final tally.

"com.eed3si9n" % "sbt-assembly" % "xx"
"org.scoverage" % "sbt-scoverage" % "xx"
"org.scalameta" % "sbt-scalafmt" % "xx"
"com.localytics" % "sbt-dynamodb" % "xx" // For DynamoDB tests

[benjben]

I share the idea that all the plugins that are there only to make the developer's life easier should be kept in his global sbt settings. Thus I'm also not in favor of using sbt-updates and sbt-revolver.

I like the idea of removing sbt-buildinfo in favor of "scalified" settings, because the result is the same and it removes a dependency.

I like sbt-tpolecat but I'm afraid that this does not get maintained for years. Also, I think that the scalac options can be quite different depending on the different libraries used for a project.

Regarding sbt-bintray, I think that we should remove it and keep only sbt-native-packager. sbt should only be in charge of creating the artifacts, be it a Docker image or a zip, it should not need to know anything about where things are published. So I'm in favor of using sbt-native-packager to package everything and then let Travis publish them wherever it wants.

What about a common plugin to generate the Scaladoc, like sbt-site?

[lukeindykiewicz]

I'm not forcing either sbt-updates, nor sbt-revolver, but I see a benefit for having revolver in each and every project. It's a common way of running it, but maybe it's oversimplified. Maybe this should be done be other things (because of docker starting locally or other things), like run.sh, which would contain everything that is necessary to run the project.

Of course it doesn't apply to libs and spark jobs and things like that.

So let's leave it out.

[chuwy]

100% agree with everything @benjben said.

Just one thing about sbt-bintray. When I said that it is not the same as sbt-native-packager, I missed the point that we're talking about apps (not libs) here. Applications can (and should) be published by Travis / release-manager. Whereas lib is a different story, we use Bintray Maven repo and JCenter to publish things to Maven Central, and this is where we need sbt-bintray.

UPD: I also have the same concerns about sbt-buildinfo and sbt-tpolecat. They have such a small functionality that it's super-simple to copy-paste them, yet they introduce risk of being unmaintained. However, I'm easy on both, having that we made some efforts to make them common across our codebases.

[peel]

IMO the ./project/plugins.sbt should contain bare minimum that is needed to compile the project. Whatever is not used in order to build the artifact or does not provide a safety net does not belong there. It takes time to be downloaded, cached, loaded and applied. Having a small set of plugins (as listed by @dilyand) to do the job at hand makes a perfect sense.

The other things can be put what stuff we recommend for developers to be using/use ourselves (be it global, or alongside the usual plugins.sbt) in engineering-resources. And there we can put all the bloops, revolvers etc. With that we'd get a pretty nice onboarding doc as well (along the lines of http://transparency.narrative.io)

Wouldn't it be also good to pull a sample CI config into the model so we can possibly set there things like scalamft failed to validate - fail the build on PR/master?