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

refactor(cache): use cmdliner groups in dune cache commands #6614

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Dec 1, 2022

We also do some cleanup of the documentation. This will make adding more subcommands easier in the future.

The (deprecated and removed) commands start and stop have also been completely removed.

closes #4471

Signed-off-by: Ali Caglayan [email protected]

@Alizter Alizter force-pushed the ps/rr/refactor_cache___use_cmdliner_groups_in_dune_cache_commands branch from cf1bfdc to ea65cea Compare December 2, 2022 11:54
Copy link
Collaborator

@snowleopard snowleopard left a comment

Choose a reason for hiding this comment

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

Can you split adding the test and doing the refactoring into separate commits, so it's easier to review the resulting changes (if any)?

@Alizter
Copy link
Collaborator Author

Alizter commented Dec 4, 2022

@snowleopard I've made #6632 which I will rebase this on so you can see the diff easier.

@snowleopard
Copy link
Collaborator

@snowleopard I've made #6632 which I will rebase this on so you can see the diff easier.

Sounds good, thanks!

@Alizter Alizter force-pushed the ps/rr/refactor_cache___use_cmdliner_groups_in_dune_cache_commands branch 2 times, most recently from df9bc09 to 1eaabdd Compare December 4, 2022 17:56
Copy link
Collaborator

@snowleopard snowleopard left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I pushed a little tweak.

We also do some cleanup of the documentation. This will make adding
more subcommands easier in the future.

closes ocaml#4471

<!-- ps-id: 60b331a0-d183-498b-9bba-5ea3d4538fb2 -->

Signed-off-by: Ali Caglayan <[email protected]>
@Alizter Alizter force-pushed the ps/rr/refactor_cache___use_cmdliner_groups_in_dune_cache_commands branch from 0eab71e to a01488a Compare December 5, 2022 21:39
@Alizter Alizter merged commit 0a66f65 into ocaml:main Dec 5, 2022
@Alizter Alizter deleted the ps/rr/refactor_cache___use_cmdliner_groups_in_dune_cache_commands branch December 5, 2022 22:05
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.

Reimplement [dune cache] command
3 participants