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

Deprecated T.command(task) for removal in Mill 0.13 #3524

Merged
merged 4 commits into from
Sep 12, 2024
Merged

Conversation

lefou
Copy link
Member

@lefou lefou commented Sep 12, 2024

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)()
 }

This change deprecates all overloads accepting a Task in favor of accepting a Result. Results can be easily acquired from a Task by calling apply() or short ().

This is also the more correct way of re-using or re-defining tasks, since directly re-used tasks may result in an incorrect tasks context, esp. an incorrect source location, which is relevant when debugging misbehaving builds.

See #3517

@lefou lefou requested review from lihaoyi, ckipp01 and lolgab and removed request for lihaoyi and ckipp01 September 12, 2024 08:01
@lefou lefou merged commit dc4eba8 into main Sep 12, 2024
23 checks passed
@lefou lefou deleted the command-from-task branch September 12, 2024 08:41
@lefou lefou added this to the 0.12.0 milestone Sep 12, 2024
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.

3 participants