Evaluate if `T.command` overload accepting a `Task` can be removed
Forgotten parenthesis on commands are a frequent cause of issue.
Here is the latest encouter:
- https://github.com/com-lihaoyi/mill/discussions/3506#discussioncomment-10618276
T.command currently has two overloads, one accepting a Result (like most of the other task factories, T, T.input, T.task and so on) and one accepting a Task. Because of the second version, command definitions like the following don't do what the user thinks they do, although they seem to work.
override def publishLocal(localIvyRepo: String): Command[Unit] = T.command {
super.publishLocal(localIvyRepo)
}
Instead of creating a new command, that depends on another command (the super-version) and returns just the result, it returns the super-command as-is.
The correct version is:
override def publishLocal(localIvyRepo: String): Command[Unit] = T.command {
super.publishLocal(localIvyRepo)()
}
override def publishLocal(localIvyRepo: String): Command[Unit] = T.command {
- super.publishLocal(localIvyRepo)
+ super.publishLocal(localIvyRepo)()
}
I wonder why we have this overload and if we should just remove it, since it causes a lot of issues. It was one of my first issues I had myself when I experimented with Mill IIRC and I helped others to identify it many many times. The rule of thumb since then is, to always use one extra pair of parenthesis compared to the definition side of the task you want to call.
This is quite obviously an issue with our API which we should fix.
I'd like to remove the overload
As a first step, I deprecated all Target creators which accept a Task (#3524). This gives helpful messages in IDEs and on CLI and prepares the removal in Mill 0.13.
Done in https://github.com/com-lihaoyi/mill/pull/4499