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.

Sign up for GitHub

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

Jump to bottom

Support dash as short for stdin #434

Merged
merged 2 commits into from
Jun 9, 2022
Merged

Support dash as short for stdin #434

merged 2 commits into from
Jun 9, 2022

Conversation

raffis
Copy link
Contributor

@raffis raffis commented May 30, 2022 •
edited
Loading

Description

Simple addition to support a dash as alias for /dev/stdin.

I would expect a cli tool to accept a simple - in order to read from the stdin stream.
For example currently I have to write:

kustomize build . | kubeaudit all -f /dev/stdin.

This pr makes it more straightforward:
kustomize build . | kubeaudit all -f -

Type of change
  • Bug fix 🐛
  • New feature ✨
  • This change requires a documentation update 📖
  • Breaking changes ⚠️
How Has This Been Tested?

Just manually, did not find any preexisting place where cli args are tested, maybe you can point to that if a test is needed.

  • Test A
  • Test B
Checklist:
  • I have 🎩 my changes (A 🎩 specifically includes pulling down changes, setting them up, and manually testing the changed features and potential side effects to make sure nothing is broken)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The test coverage did not decrease
  • I have signed the appropriate Contributor License Agreement

@ghost
Copy link

ghost commented May 30, 2022

Thanks for opening this pull request! Please check out our contributing guidelines and sign the CLA.

@ghost ghost added the core label May 30, 2022
@genevieveluyt
Copy link
Contributor

Hi @raffis, thanks for your contribution, this makes sense to me! Could you please update the documentation for the -f flag as well?

@genevieveluyt
Copy link
Contributor

I'll try to take a closer look at why the test is failing (hopefully by end of next week). Doesn't seem to be related to your changes

@ghost ghost added the readme label Jun 2, 2022
@raffis
Copy link
Contributor Author

raffis commented Jun 2, 2022

Hi @raffis, thanks for your contribution, this makes sense to me! Could you please update the documentation for the -f flag as well?

Docs updated 👍🏻

@genevieveluyt
Copy link
Contributor

Hi @raffis , tests should be fixed as of #426. Could you please rebase your branch on main?

@raffis
Support dash as short for stdin
2d80820 
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
@raffis
document dash stdin
5abdeb8 
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
@genevieveluyt
Copy link
Contributor

Fyi I'm just waiting to get all the dependency updates and bug fixes into a patch release and then I will merge this and do a minor release! Thanks for your patience 🙏

@genevieveluyt genevieveluyt merged commit 8a2ee04 into Shopify:main Jun 9, 2022
@ghost
Copy link

ghost commented Jun 9, 2022

Congrats on merging your first pull request, keep em coming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@genevieveluyt genevieveluyt genevieveluyt approved these changes

Assignees
No one assigned
Labels
core readme
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@raffis @genevieveluyt