-
Notifications
You must be signed in to change notification settings - Fork 508
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
📖 Remove erroneous ref to CSV output #1813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Codecov Report
@@ Coverage Diff @@
## main #1813 +/- ##
=======================================
Coverage 51.27% 51.27%
=======================================
Files 80 80
Lines 6797 6797
=======================================
Hits 3485 3485
Misses 3081 3081
Partials 231 231 |
Integration tests success for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nits on the language.
In addition to mentioning the format options, we should specify what default
actually means (outputting as a table).
Could you mention this in the flags as well?
Integration tests success for |
Integration tests success for |
Integration tests success for |
Stale pull request message |
@justaugustus Done. Thanks. |
Integration tests success for |
Integration tests success for |
Stale pull request message |
I realize this isn't the most exciting PR maintainers might dream of but I would advise them to be a bit more diligent in merging PRs from the community. It is not a very pleasant experience to have to keep rebasing your PRs because it is being left unattended by the maintainers. @justaugustus could you please help? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this isn't the most exciting PR maintainers might dream of but I would advise them to be a bit more diligent in merging PRs from the community. It is not a very pleasant experience to have to keep rebasing your PRs because it is being left unattended by the maintainers.
Thanks for the PR, @lehors!
As a note, several of the maintainers are preparing for conferences and other events, hence the delayed reviews.
Integration tests success for |
Signed-off-by: Arnaud J Le Hors [email protected]
What kind of change does this PR introduce?
docs update
What is the current behavior?
README claims that CSV output format is supported.
What is the new behavior (if this is a feature change)?**
README only lists default and json as output formats.
NA.
Which issue(s) this PR fixes
NONE
Special notes for your reviewer
I actually happen to regret the drop of CSV output support but at least the documentation should be consistent with the code.
Does this PR introduce a user-facing change?
For user-facing changes, please add a concise, human-readable release note to
the
release-note
Yes, the README no longer misleads the user.