Skip to content

✨ Separate check from policies for the Vulnerabilities check#1532

Merged
justaugustus merged 6 commits intoossf:mainfrom
laurentsimon:feat/raw-osv
Jan 26, 2022
Merged

✨ Separate check from policies for the Vulnerabilities check#1532
justaugustus merged 6 commits intoossf:mainfrom
laurentsimon:feat/raw-osv

Conversation

@laurentsimon
Copy link
Copy Markdown
Contributor

@laurentsimon laurentsimon commented Jan 26, 2022

checker/raw_results.go: add structure for results
checks/vulnerabilities.go: rewrite fr policy seperation
checks/raw/vulnerabilities.go: data retrieval
checks/evaluation/vulnerabilities.go: score calculation
pkg/json_raw_results.go: displays the results.

Refactor the Vulnerabilities check by separating the data retrieval and policy (score) evaluation.
This will allow users to create their own policies based on the raw (more structured) results, see ttps://github.com//issues/1245

No breaking changes.

Copy link
Copy Markdown
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

@laurentsimon -- Overall looks good, just a few nits.

Can you also tighten up the commit messages?

<area>: <what's changing>

<why it's changing>

<footer/signoff>

ref: https://cbea.ms/git-commit/

Comment thread checks/evaluation/vulnerabilities.go
Comment thread checker/raw_result.go Outdated
Comment thread checks/vulnerabilities.go
@naveensrinivasan
Copy link
Copy Markdown
Member

@laurentsimon -- Overall looks good, just a few nits.

Can you also tighten up the commit messages?

<area>: <what's changing>

<why it's changing>

<footer/signoff>

ref: https://cbea.ms/git-commit/

Great suggestion, but this is not a blocker IMO. We have been following a process for our commits and it is working.

We don't stress for sign-off. If we want to bring such changes, can you please create an issue?

@laurentsimon
Copy link
Copy Markdown
Contributor Author

@laurentsimon -- Overall looks good, just a few nits.

Can you also tighten up the commit messages?

<area>: <what's changing>

do I need to write <area> because we're going to use a tool for release notes? Or is this just a placeholder?

<why it's changing>

same question as above.

<footer/signoff>


ref: https://cbea.ms/git-commit/

what should I add in the footer/signoff?

@laurentsimon laurentsimon changed the title [RAW]: Vulnerabilities check support Separate check from policies for the Vulnerabilities check Jan 26, 2022
@laurentsimon
Copy link
Copy Markdown
Contributor Author

@laurentsimon -- Overall looks good, just a few nits.

Can you also tighten up the commit messages?

<area>: <what's changing>

<why it's changing>

<footer/signoff>

ref: https://cbea.ms/git-commit/

tell me if the new description is fine or not, thanks.

@laurentsimon laurentsimon temporarily deployed to integration-test January 26, 2022 18:31 Inactive
@justaugustus
Copy link
Copy Markdown
Member

We don't stress for sign-off. If we want to bring such changes, can you please create an issue?

Sure, that's a larger discussion: #1533

do I need to write <area> because we're going to use a tool for release notes? Or is this just a placeholder?

This is more for code hygiene than anything else. Ideally, you can interrogate a commit message and understand exactly why a change happened.

Imagine you're debugging a regression and looking through the tree for a code change that was maybe not as innocuous as you expected.

Would you prefer to see:

linter

OR

checks/raw: Fix `errcheck` lint warnings

Another example:

githubrepo updates

OR

clients/githubrepo: Refactor clients to accept `log.Logger`

@github-actions
Copy link
Copy Markdown

@david-a-wheeler
Copy link
Copy Markdown
Contributor

Given the previous comments from @swinslow , we might want to change all copyright statements to something like:

// Copyright Security Scorecard Authors

See Copyright Notices in Open Source Software Projects. I believe the legal requirement for the copyright statement (with the date) ended in the US in 1976 :-).

@laurentsimon laurentsimon changed the title Separate check from policies for the Vulnerabilities check ✨ Separate check from policies for the Vulnerabilities check Jan 26, 2022
@laurentsimon
Copy link
Copy Markdown
Contributor Author

friendly ping for LGTM.

Copy link
Copy Markdown
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Thanks @laurentsimon!

@naveensrinivasan
Copy link
Copy Markdown
Member

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.

5 participants