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

Maximum severity rating for each Group object in JSON output #805

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

AppleGamer22
Copy link
Contributor

Signed-off-by: Omri Bornstein <omribor@gmail.com>
Signed-off-by: Omri Bornstein <omribor@gmail.com>
Copy link
Collaborator

@G-Rath G-Rath 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 - would be nice to have this change expressed in a test somehow if possible

@@ -4,6 +4,7 @@ import (
"sort"
"strings"

out "github.com/google/osv-scanner/internal/output"
Copy link
Collaborator

Choose a reason for hiding this comment

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

happy for this to be done in a follow up pr sometime, but ideally it would be better to rename the variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, I'll rename the output variables to results.

…ents name conflict with internal output package

Signed-off-by: Omri Bornstein <omribor@gmail.com>
Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Thanks! Can you also replace the other places MaxSeverity is being called to use what's now being stored in groups?

I believe it's called in githubannotations.go and table.go

… struct property

Signed-off-by: Omri Bornstein <omribor@gmail.com>
@AppleGamer22
Copy link
Contributor Author

@another-rex @G-Rath thank you both for the thoughtful feedback.

@AppleGamer22
Copy link
Contributor Author

I'll get to writing the MaxSeverity test now...

Signed-off-by: Omri Bornstein <omribor@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (85563d9) 79.98% compared to head (02af4a6) 58.96%.
Report is 4 commits behind head on main.

Files Patch % Lines
pkg/osvscanner/vulnerability_result.go 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #805       +/-   ##
===========================================
- Coverage   79.98%   58.96%   -21.02%     
===========================================
  Files          91      116       +25     
  Lines        6214     8442     +2228     
===========================================
+ Hits         4970     4978        +8     
- Misses       1040     3261     +2221     
+ Partials      204      203        -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@AppleGamer22 AppleGamer22 requested a review from G-Rath March 2, 2024 04:05
@another-rex another-rex merged commit 07cbeff into google:main Mar 4, 2024
11 checks passed
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.

None yet

4 participants