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

Add --format="text" option to vip logs #2121

Merged
merged 4 commits into from
Nov 26, 2024

Conversation

owen-cole
Copy link
Contributor

@owen-cole owen-cole commented Nov 23, 2024

Description

In #2089 the table option was formatted to meet a style guide's requirements. I see the default option was actually previously table but not formatted as a table so this change makes sense.

The previous basic text format was nice to have especially when using the --follow flag and matches what is described here https://docs.wpvip.com/logs/runtime-logs/cli/

The docs mention an option for formatting called text and this PR enables that option and restores the previous formatting of the logs under that option:
https://github.com/Automattic/vip-cli/blame/19c65e4758b30be5d9708ae807f754f2a6883ef1/src/bin/vip-logs.js#L146-L151

Pull request checklist

New release checklist

Steps to Test

Outline the steps to test and verify the PR here.

Example:

  1. Check out PR.
  2. Run npm run build
  3. Run ./dist/bin/vip-cookies.js nom
  4. Verify cookies are delicious.

@owen-cole owen-cole marked this pull request as ready for review November 23, 2024 19:49
@@ -243,7 +249,7 @@ command( {
`The maximum number of entries to return. Accepts an integer value between 1 and 5000 (defaults to ${ LIMIT_DEFAULT }).`
)
.option( 'follow', 'Output new entries as they are generated.' )
.option( 'format', 'Render output in a particular format. Accepts “csv”, and “json”.', 'table' )
.option( 'format', 'Render output in a particular format. Accepts “csv”, “json”, and “text”.', 'table' )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.option( 'format', 'Render output in a particular format. Accepts “csv”, “json”, and “text”.', 'table' )
.option( 'format', 'Render output in a particular format. Accepts “csv”, “json”, “table”, and “text”.', 'table' )

@yolih what do you think?

@sjinks sjinks requested a review from yolih November 24, 2024 19:14
Copy link

sonarcloud bot commented Nov 26, 2024

@sjinks sjinks merged commit fbc1415 into Automattic:trunk Nov 26, 2024
1 check passed
@sjinks sjinks mentioned this pull request Nov 26, 2024
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.

3 participants