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

pkg: Install all the dev-tools that are available #11032

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

moyodiallo
Copy link
Collaborator

This is supposed to close #11001.

But instead of having dune tools install I wonder if dune build @pkg-tools could be a better alternative, because this is equivalent building/invoking some aliases.

@Leonidas-from-XIV
Copy link
Collaborator

I wonder if dune build @pkg-tools could be a better alternative, because this is equivalent building/invoking some aliases.

That's a very good point. Maybe dune build @install-dev-tools (to match @install and `@install-pkg from #11046) would make sense. I've added @maiste to weight in on this.

@maiste
Copy link
Collaborator

maiste commented Nov 7, 2024

I've added @maiste to weight in on this.

I would advocate for the opposite here. The aliases make sense when they are related to building targets in the context of Dune. Here, we are talking about development tools, so I'm afraid it would break the semantics as they are external tools. Also, in terms of accessibility, it would be easier for users to get access to the command than to the alias as with @gridbugs work, they have access to auto-completion.

My recommendation would be to stick to the command instead of the alias, but in the end, it is just my opinion 😄


let lock_tools (tools : Dune_pkg.Dev_tool.t list) =
List.map tools ~f:(function
| Dune_pkg.Dev_tool.Odoc -> lock_odoc ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a type annotation so this prefix is not needed?

List.map dev_tools_path ~f:Action_builder.path |> Action_builder.all_unit)
in
match result with
| Error `Already_reported -> raise Dune_util.Report_error.Already_reported
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder, shouldn't this be Code_error.raise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If there is an error, it is already reported during the build and Code_error.raise is about raising with a message.

Signed-off-by: Alpha DIALLO <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg: a command line that installs all the dev-tools at once.
3 participants