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 --log-level option to vti diagnostics #2752

Merged
merged 8 commits into from
Mar 8, 2021

Conversation

yukukotani
Copy link
Contributor

@yukukotani yukukotani commented Mar 3, 2021

Resolves #2723.

vti diagnostics --log-level ERROR to ignore warns and print only errors.

$ vti diagnostics --help
Usage: vti diagnostics [options] [workspace]

Print all diagnostics

Options:
  -l, --log-level <logLevel>  Log level to print (choices: "ERROR", "WARN", "INFO", "HINT", default: "WARN")
  -h, --help                  display help for command

Copy link
Member

@yoyo930021 yoyo930021 left a comment

Choose a reason for hiding this comment

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

Thank you.

vti/src/cli.ts Outdated Show resolved Hide resolved
vti/src/cli.ts Outdated Show resolved Hide resolved
@yukukotani yukukotani changed the title add error-only option to vti diagnostics add --log-level option to vti diagnostics Mar 7, 2021
Copy link
Member

@yoyo930021 yoyo930021 left a comment

Choose a reason for hiding this comment

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

LGTM. Only some small problems.

vti/src/cli.ts Outdated
)
.action(async (workspace, options) => {
const logLevelOption: unknown = options.logLevel;
if (
Copy link
Member

Choose a reason for hiding this comment

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

Why don't use logLevels from import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yoyo930021 Because logLevels type is readonly ["ERROR", "WARN", "INFO", "HINT"], we cannot call logLevels.includes with unknown value.

related issue: microsoft/TypeScript#26255

Copy link
Member

Choose a reason for hiding this comment

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

We can use as keyword or is function to force it.
Repeated text will cause follow-up maintenance problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yoyo930021 ok. I've removed repeated text. e905cb2

vti/src/commands/diagnostics.ts Outdated Show resolved Hide resolved
@yukukotani yukukotani requested a review from yoyo930021 March 8, 2021 13:52
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.

Print only errors with VTI
2 participants