Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
make a scalafmt worker a singleton
  • Loading branch information
rockjam committed May 2, 2018
commit 34587b991a54b387de6868276cf0b26efbdce8d6
16 changes: 8 additions & 8 deletions scalalib/src/mill/scalalib/scalafmt/ScalafmtModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@ package mill.scalalib.scalafmt

import ammonite.ops._
import mill._
import mill.define.{Command, Sources, Worker}
import mill.define.{Command, Sources}
import mill.scalalib._

trait ScalafmtModule extends ScalaModule {

def reformat(): Command[Unit] = T.command {
worker().reformat(
filesToFormat(sources()),
scalafmtConfig().head,
scalafmtDeps().map(_.path)
)
ScalafmtWorkerModule
.worker()
.reformat(
filesToFormat(sources()),
scalafmtConfig().head,
scalafmtDeps().map(_.path)
)
}

def scalafmtVersion: T[String] = "1.5.1"
Expand All @@ -27,8 +29,6 @@ trait ScalafmtModule extends ScalaModule {
)
}

def worker: Worker[ScalafmtWorker] = T.worker { new ScalafmtWorker() }

private def filesToFormat(sources: Seq[PathRef]) = {
for {
pathRef <- sources if exists(pathRef.path)
Expand Down
9 changes: 8 additions & 1 deletion scalalib/src/mill/scalalib/scalafmt/ScalafmtWorker.scala
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package mill.scalalib.scalafmt

import ammonite.ops.{Path, exists}
import mill.{Agg, PathRef}
import mill._
import mill.define.{Discover, ExternalModule, Worker}
import mill.modules.Jvm
import mill.util.Ctx

import scala.collection.mutable

object ScalafmtWorkerModule extends ExternalModule {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't immediately clear that worker must live in a separate module to be a single one in a build.
Also, I copy-pasted extends ExternalModule from scala and scalajs workers. Why should it be an ExternalModule, not just a Module ?
When I tried extends Module complilation failed with:

[error] /Users/rockjam/projects/mill/scalalib/src/mill/scalalib/scalafmt/ScalafmtWorker.scala:11:54: Modules, Targets and Commands can only be defined within a mill Module
[error] private[scalafmt] object ScalafmtWorker extends mill.Module {

Copy link
Member

Choose a reason for hiding this comment

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

@rockjam being an ExternalModule allows a task myTask to be run via mill.scalalib.scalafmt.ScalafmtWorkerModule/myTask. Normal Module objects must be defined in the build.sc file and be run via their object selector from the root of build.sc.

If you want to let people run ScalaFmt without needing to extend anything, you could make a task in the ExternalModule that lets people run e.g. mill.scalalib.scalafmt.ScalafmtWorkerModule/fmt __.sources to format the sources of all their ScalaModules. See how the mill.scalalib.PublishModule/publishAll command works as an example.

As for being separate Mill submodules, we only needed to put things like ScalaWorker/ScalaJSWorker in there because we needed code that runs with some heavyweight dependencies on the classpath that might collide (e.g. different Scala.js versions): the dependencies are isolated within a classloader and interacted with via reflection. If you don't have any heavyweight dependencies, or you do but you can just interact with them directly via reflection/subprocesses, then there's no need to isolate them in a separate Mill build module

def worker: Worker[ScalafmtWorker] = T.worker { new ScalafmtWorker() }

lazy val millDiscover = Discover[this.type]
}

private[scalafmt] class ScalafmtWorker {
private val reformatted: mutable.Map[Path, Int] = mutable.Map.empty
private var configSig: Int = 0
Expand Down