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

feat: OS CLI output part I - update severity headline +test #3437

Closed
wants to merge 2 commits into from

Conversation

florindumitrascu
Copy link
Contributor

@florindumitrascu florindumitrascu commented Jul 5, 2022

What does this PR do?

Improve snyk-open-source CLI output as follows:

  • Severity and Title should be the only content on the line, similar to the web/ide
  • [Severity] should be the first, in front of the title, similar to web/ide
  • Remove the word ‘severity’ and only keeping [Critical], [High], [Medium], [Low]
  • Proper color usage (e.g. Green shouldn’t be used for section titles)

Where should the reviewer start?

src/lib/formatters/remediation-based-format-issues.ts

How should this be manually tested?

Run 'snyk test' command on a local project and compare output formatting with the attached screenshot

Any background context you want to provide?

What are the relevant tickets?

TUN-269

Screenshots

Screenshot 2022-07-27 at 15 20 44

Additional questions

@florindumitrascu florindumitrascu requested review from a team as code owners July 5, 2022 10:54
@florindumitrascu florindumitrascu marked this pull request as draft July 5, 2022 11:02
) +
reachabilityText +
'\n ' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering you should use \t instead of to be more explicit, if the intention is for it to be tabbed?

danielroymoore
danielroymoore previously approved these changes Jul 15, 2022
Copy link
Contributor

@danielroymoore danielroymoore left a comment

Choose a reason for hiding this comment

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

Looks good to me, left a comment that may or may not be relevant, commits also needs squashing prior to merge.

- Severity and Title should be the only content on the line, similar to the web/ide
- [Severity] should be the first, in front of the title, similar to web/ide
- Removing the word ‘severity’ and only keeping [Critical], [High], [Medium], [Low]
- Proper color usage (e.g. Green shouldn’t be used for section titles)
@florindumitrascu florindumitrascu force-pushed the feat/os-cli-output+test branch from 5103fa8 to 305de41 Compare July 27, 2022 12:00
@florindumitrascu florindumitrascu changed the title feat: OS CLI output - add newline after severity feat: OS CLI output - update severity headline Jul 27, 2022
@florindumitrascu florindumitrascu changed the title feat: OS CLI output - update severity headline feat: OS CLI output part I - update severity headline Jul 27, 2022
@florindumitrascu florindumitrascu changed the title feat: OS CLI output part I - update severity headline feat: OS CLI output part I - update severity headline +test Jul 27, 2022
Merge from branch 'master' into 'feat/os-cli-output+test'
@florindumitrascu florindumitrascu force-pushed the feat/os-cli-output+test branch from d9f28cf to e7a7b04 Compare July 27, 2022 14:32
@thisislawatts
Copy link
Member

🧹 Closing as stale. No activity in more than 1 year.

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.

4 participants