fix: sort descending by CVSS scores first #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jamestelfer
force-pushed
the
sort-by-score
branch
from
8d79827 to
1c56612
Compare
jamestelfer
force-pushed
the
support-cvss3
branch
from
bd845fc to
7f19863
Compare
It requires the use of another tool, and is more opinionated than worthwhile.
Applies feedback on the CVSS2 version to this one.
The inline expectations are getting large, and the upcoming change includes a structure with private internals.
jamestelfer
force-pushed
the
sort-by-score
branch
from
1c56612 to
84ccc66
Compare
Allows for consistent representation and easier sorting.
jamestelfer
force-pushed
the
sort-by-score
branch
from
84ccc66 to
57d2e5b
Compare
ctgardner
reviewed
Nov 30, 2023
|
|
||
| Ignore *findingconfig.Ignore | ||
| } | ||
|
|
||
| type CVSSScore struct { | ||
| Score *decimal.Decimal |
There was a problem hiding this comment.
Interesting. Why use a pointer to a Decimal?
There was a problem hiding this comment.
Because html/template understands nil more easily than Decimal{} making the rendering logic easier. Also it helps make it clear when the score is not supplied versus valued. I originally wrote this without the pointer, switching only when it came to rendering.
Co-authored-by: Callum Gardner <callum.gardner@cultureamp.com>
Co-authored-by: Callum Gardner <callum.gardner@cultureamp.com>
Co-authored-by: Callum Gardner <callum.gardner@cultureamp.com>
|
For some reason my comments via vscode were added to a review rather than just answering the thread. TIL |
ctgardner
approved these changes
Nov 30, 2023
Yep, it's a limitation of the VS Code extension. It's still my preferred method of reviewing PRs though, including this one. 😉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tip
This PR reads commit-by-commit
Based on CVSS3 support added in #26.
Leads to more readily understandable annotations as the most critical are rendered higher.
CVSS3 and CVSS2 schemes are sorted separately because their scoring outcomes are not comparable. This means that in longer lists, CVSS2 results are pushed down the page, however considering that AWS is moving away from CVSS2 and few images have such a massive pile of issues it's probably the right trade-off.
Sort is by:
Example:
