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: 197 add input-file flag validation for validate command #205

Merged

Conversation

kislerdm
Copy link
Contributor

Closes #197

What changed

  • Added validation of the flag -f for the validate command.

Why do we need it

  • UX improvement through early failure and clear error message.

Questions

  • Why it was decided to use cobra for such a simple CLI?
  • Should behavioural tests be considered for the CLI flags? For context, automated tests for the f flag won't give much value now, but assuming long-term vision (based on the cobra framework), behavioural tests for all flags/commands might be beneficial.

Signed-off-by: Dmitry Kisler <admin@dkisler.com>
Copy link

netlify bot commented Jan 11, 2024

Deploy Preview for lula-docs canceled.

Name Link
🔨 Latest commit cad92bb
🔍 Latest deploy log https://app.netlify.com/sites/lula-docs/deploys/65a080b13e985d0008f1b5e6

@kislerdm kislerdm changed the title Fixed input file flag validation fix: 197 add input-file flag validation for validate command Jan 12, 2024
Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

Why it was decided to use cobra for such a simple CLI?

No particularly strong opinion outside of alignment with other closely related open source projects and potential command vendoring options in the future.

Should behavioural tests be considered for the CLI flags? For context, automated tests for the f flag won't give much value now, but assuming long-term vision (based on the cobra framework), behavioural tests for all flags/commands might be beneficial.

I love the idea. We're working on more command testing that would align well to that benefit

Nonetheless - Very much appreciate this contribution to Lula!

@brandtkeller brandtkeller merged commit 0017a60 into defenseunicorns:main Jan 12, 2024
7 checks passed
@kislerdm kislerdm deleted the 197-fix-input-flags-validation branch January 12, 2024 00:39
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.

Fix validation error for not providing input file flag
2 participants