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

Make Mill commands require named CLI parameters by default #3431

Merged
merged 9 commits into from
Aug 29, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Aug 29, 2024

This aligns the semantics with MainArgs library defaults. Previously we grandfathered in the "positional by default" behavior because Mill pre-dates mainargs, with original behavior inherited from Ammonite. But most CLIs do not allow keyword args to be naively replaced by positional args, and MainArgs thus chose to have such behavior as opt-in. Mill should follow suite

Added a flag --allow-positional-command-args to fall back to the old behavior, and covered this behavior with example tests, docs, and updated the changelog.

This is a binary-compatible but semantically-incompatible change, so it should go into 0.12.0. It's a pretty painful breaking change, but it's definitely the right thing to do long term, so better to get it over with. People can also add --allow-positional-command-args to their ./mill file if they want to preserve the old behavior in perpetuity

@lihaoyi lihaoyi requested a review from lefou August 29, 2024 02:14
@lihaoyi lihaoyi marked this pull request as ready for review August 29, 2024 02:23
@lihaoyi lihaoyi requested a review from lolgab August 29, 2024 05:03
@lefou
Copy link
Member

lefou commented Aug 29, 2024

Looks like a good candidate for persistent local configuration.

@lihaoyi lihaoyi merged commit d30f2ca into com-lihaoyi:main Aug 29, 2024
33 checks passed
@lefou lefou added this to the 0.12.0 milestone Aug 29, 2024
lihaoyi added a commit that referenced this pull request Sep 3, 2024
…al (#3455)

These are commands that probably do not expect named params that I found
in a cursory audit, that would otherwise be broken by
#3431. We can annotate others as
we find them
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.

2 participants