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

Extend version task with scala/java version. (was: add --version option) #491

Closed
lefou opened this issue Nov 30, 2018 · 9 comments · Fixed by #760
Closed

Extend version task with scala/java version. (was: add --version option) #491

lefou opened this issue Nov 30, 2018 · 9 comments · Fixed by #760
Labels
good-first-issue The fix/solution for the issue is considered easy. A good starting point for newbies.
Milestone

Comments

@lefou
Copy link
Member

lefou commented Nov 30, 2018

This might be useful, e.g. in CI builds and is in general a best practice.

It should also include the used Java and Scala version.

@lefou lefou added the good-first-issue The fix/solution for the issue is considered easy. A good starting point for newbies. label Nov 30, 2018
@lihaoyi
Copy link
Member

lihaoyi commented Dec 2, 2018

Would this be different from the mill version command that already exists?

@lefou
Copy link
Member Author

lefou commented Dec 3, 2018

It depends. I forgot about the version command, when opening this issue, but this issue also requests to include the used java and scala version, possibly also library versions which are hard-coupled with mill (e.g. os-lib).

Having a command for version is a bit weird. All other apps I know use a command line option for it. Also, the version command lies currently in the task namespace, which IMHO should be in control of the mill user. An attribute, which it shares with all the other "built-in" commands like "all", "inspect", ... I'd like to move all those to command line options, or at least into a separate namespace, e.g. mill.version.

@lihaoyi
Copy link
Member

lihaoyi commented Dec 4, 2018

The current convention is that things which apply to "all tasks" are implemented as flags, and everything else is implemented as tasks. So e.g. --watch is a flag, and --parallel would be a flag, etc.

Currently we have some weirdness in that tasks such as version or clean still end up creating an out/ folder, writing stuff to meta.json, and needlessly running the dependency resolution logic, but I think we can get rid of that stuff and still leave them as tasks.

There's a bit of namespace pollution, but I don't think it's that big a deal. It's also nice because users have the ability to add their own top-level tasks/commands that are on-par with the builtins, in a straightforward way.

@lefou
Copy link
Member Author

lefou commented Dec 4, 2018

Unfortunately, namespace pollution is an issue in mill. I usually have some top-level "build", "test", "publish" tasks to make common tasks easy, but even "build", which is not a default mill command, collides, when running mill -i. And --no-default-predef option doesn't help. So nowadays, my top-level tasks/commands all start with a _, which isn't comfortable, and even doesn't match well with mills wildcard expressions. That's why I'd like to have a separate namespace for built-ins.

@lihaoyi
Copy link
Member

lihaoyi commented Dec 7, 2018

build is not a task; not sure if you can define a build task now, but if not we can probably make it work. Then the enclosing build object can still be accessed via build.this. As mentioned above, we can probably clean up the existing default tasks to not create out folders, initialize log files, and other things. We can tweak mill version to include java/scala versions.

It comes down to a value judgement then:

  • Should we prioritize your top-level tasks, and put the builtins in a namespace,
  • Or prioritize the builtins and you can put your custom tasks in a namespace,
  • Or, since I think the only name likely to collide is version, to rename your version to something else like artifactVersion, publishVersion or deployVersion

Let's go with renaming your version task for now. I'd like to avoid scattering things across different namespaces, and would prefer having a more crowded namespace than having lots of flags with their own namespace but less clear syntax or semantics.

@lefou
Copy link
Member Author

lefou commented Dec 7, 2018

The name collision with build occurs, when running mill -i.

$ mill -i
Loading...
Compiling /home/lefou/work/opensource/mill-osgi/(console)
(console):7: reference to build is ambiguous;
it is imported twice in the same scope by
import build._
and import ammonite.predef.build
  build.millSelf.get,
  ^
(console):8: reference to build is ambiguous;
it is imported twice in the same scope by
import build._
and import ammonite.predef.build
  build.millDiscover,
  ^
Compilation Failed

@lefou
Copy link
Member Author

lefou commented Dec 7, 2018

The build command in the corresponding build.sc

/** Build JARs. */
def build() = T.command {
  core.jar()
}

@lefou lefou changed the title Add --version option Extend version task with scala/java version. (was: add --version option) Dec 7, 2018
@lefou
Copy link
Member Author

lefou commented Jan 20, 2020

I regularly see users to want to use a mill --version. It's a very common option, to not say "standard option". I think we should provide that special option, even though we can implement it as a task. We can still keep the version task to make that value accessible in a script.

Here are some reasons for the --version addition:

  • mill --version would return instantly with a single printed value
  • no need to start a mill server in the backgroud just to check that values and act upon it's value
  • no risk of a failure without seeing the version in case of some issues in the build.sc
  • no other spurious error/warning message in case there is no build.sc
  • no risk of failure in case there is no internet connection
  • and so on

@Ammonite-Bot
Copy link
Collaborator

Ammonite-Bot commented Jan 20, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue The fix/solution for the issue is considered easy. A good starting point for newbies.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants