Add -p short alias for --param in luca run - #106
Merged
Conversation
Lets `luca run` parameter values be passed more tersely, e.g. `luca run build -p flavor=release -p upload=true`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The original ask was to improve the UX of
luca runso parameters could be passed without the--paramflag at all, e.g.:instead of the current:
We explored a few options before landing on this PR's approach:
key=valuepositional arguments (the original ask). Implemented as an@Argumentarray trailing the existing optional<name>argument. This reads naturally, but ArgumentParser fills positional arguments greedily in declaration order: with<name>as an optional scalar positional followed by an array positional, a barekey=valuetoken given while using--file(which is mutually exclusive with<name>) would get wrongly assigned tonameinstead of to the params array — e.g.luca run --file x.yml flavor=releasewould setname = "flavor=release". This is a real, silent-failure-shaped edge case, not just a corner case to document away.validate()/run()to notice whennameitself looks likeKEY=VALUEand--fileis set, and reclassify it as a param. This resolves the ambiguity but adds parsing "magic" that's surprising and harder to reason about/test.-pas a short-name alias for--param(chosen). No positional parsing ambiguity, no magic, fully backward compatible with existing scripts/CI using--param. Slightly less terse than the original ask, but a much safer change:Given the risk profile of option 1/2 versus the low-risk, high-value option 3, we went with the
-palias.Type of Change
How Has This Been Tested?
swift build, thenluca run --file test.yml -p flavor=release -p upload=true --dry-run, verified both params resolved as overrides)RunCommandargument parsing; behavior verified manually and via full test suite)swift testsuite passes (581 tests)Screenshots / Demo (if applicable)
N/A (CLI flag change)
Checklist
CI Considerations
Breaking Changes?
Additional Notes
--paramcontinues to work exactly as before;-pis purely additive.