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

feat(run): Allow application args overriding from CLI #1694

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

LucaSeri
Copy link
Contributor

@LucaSeri LucaSeri commented Jun 2, 2024

Passing arguments from the CLI directly overrides any commands from the Kraftfile or Dockerfile. The first argument of kraft run is the context and any subsequence argument is considered an application argument.

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran make fmt on your commit series before opening this PR;
  • Updated relevant documentation.

Description of changes

Closes: #1614

@LucaSeri LucaSeri requested a review from nderjung June 3, 2024 08:32
Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

All good here. Thanks!

Reviewed-by: Cezar Craciunoiu [email protected]

Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

I think it is better to inject this logic across all runners as these have context-awareness of args[0] and whether it is used and whether, therefore, args[1:] represents application arguments or it is args.

@craciunoiuc craciunoiuc added this to the KraftKit v0.9.0 milestone Jun 19, 2024
Passing arguments from the CLI directly overrides any
commands from the Kraftfile or Dockerfile. The first argument
of kraft run is the context and any subsequence argument
is considered an application argument.

Signed-off-by: Luca Seritan <[email protected]>
@LucaSeri
Copy link
Contributor Author

@nderjung turns out all the runners besides one(the one I was mostly using) were already doing this, and in the last runner, the runner.args just happened to be omited.

@LucaSeri LucaSeri requested a review from nderjung June 19, 2024 13:02
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.

Kraftfile cmd not added as kernel argument
3 participants