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

fix: kamel run -o incompatible with --dev #3219

Merged
merged 3 commits into from
Apr 20, 2022

Conversation

KuthumiPepple
Copy link
Contributor

Closes #3195
@astefanutti @squakez I included a test first for the fix. For the fix, I added a check for both flags. If both flags are present, an error is returned with the error message written to the error writer. Please review and let me know if there are any more things I could do.

Release Note

NONE

pkg/cmd/run.go Outdated
@@ -243,6 +243,10 @@ func (o *runCmdOptions) validate() error {
return err
}

if o.OutputFormat != "" && o.Dev {
return errors.New("option '-o' is not compatible with '--dev'")
Copy link
Contributor

Choose a reason for hiding this comment

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

We better keep using the fmt package as we're doing for the rest of errors, ie return fmt.Errorf(invalid label specification %s. Expected "<labelkey>=<labelvalue>", label)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'd use the fmt package

@KuthumiPepple
Copy link
Contributor Author

I have implemented the change

pkg/cmd/run.go Outdated
@@ -243,6 +243,10 @@ func (o *runCmdOptions) validate() error {
return err
}

if o.OutputFormat != "" && o.Dev {
return fmt.Errorf(`invalid dev specification "%t". Expected "--dev=false"`, o.Dev)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it won't be clear for the user why this is invalid. I guess something like cannot use --dev with -o/--output options would be clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll amend and force push

@oscerd
Copy link
Contributor

oscerd commented Apr 20, 2022

Please rebase on main, so it will be aligned and test will pass.

@KuthumiPepple
Copy link
Contributor Author

Please rebase on main, so it will be aligned and test will pass.

Done

@oscerd oscerd merged commit ef29f63 into apache:main Apr 20, 2022
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.

Kamel run -o incompatible with --dev
3 participants