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

✨ Feature: Dependency-diff Visualization in the Scorecard Action (version 0 - part 1) #780

Closed
wants to merge 74 commits into from

Conversation

aidenwang9867
Copy link

@aidenwang9867 aidenwang9867 commented Jul 28, 2022

What kind of change does this PR introduce?

Introduce the Dependency-diff Visualization in the Scorecard Action (version 0) feature.

What is the new behavior (if this is a feature change)?**

This is a new mode of Scorecard Action that can run on a pull request, analyze the dependency-diffs, surface the Scorecard checks for them and visualize them in the (1) PR comment and (2) check run annotations. This PR only introduces (1), (2) will be given in a separate one.

  • Tests for the changes have been added (for bug fixes/features)
    TODO (#issue number): add e2e tests for this feature to make sure the run won't crash on a pull request.

Which issue(s) this PR fixes

Fixes #772 (the dependency package deps.dev page will be given in the PR comment if the page exists)

Does this PR introduce a user-facing change?

Yes.

The Scorecard Action now can run on a pull request to surface Scorecard check results in the check run annotations and visualize the dependency-diffs in the pull request comment.

@aidenwang9867 aidenwang9867 marked this pull request as draft July 28, 2022 06:17
@aidenwang9867 aidenwang9867 marked this pull request as ready for review July 28, 2022 06:18
@aidenwang9867 aidenwang9867 changed the title ✨ Feature: Dependency-diff Visualization in the Scorecard Action (version 0) ✨ Feature: Dependency-diff Visualization in the Scorecard Action (version 0 - part 1) Jul 28, 2022
Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

Thanks, @aidenwang9867!

Could you please add unit tests? It adds reliability.

It is critical that we have tests.

@aidenwang9867
Copy link
Author

aidenwang9867 commented Jul 28, 2022

Thanks, @aidenwang9867!

Could you please add unit tests? It adds reliability.

It is critical that we have tests.

Of course! Should I change this to a draft PR for now / add the tests in a follow-up PR? It might take me some extra effort to have the tests covered since I'm not very familiar with testing for an action app.

@naveensrinivasan
Copy link
Member

Thanks, @aidenwang9867!
Could you please add unit tests? It adds reliability.
It is critical that we have tests.

Of course! Should I change this to a draft PR for now / add the tests in a follow-up PR? It might take me some extra effort to have the tests covered since I'm not very familiar with testing for an action app.

The tests are standard go tests. Can we add unit tests for this PR? Probably certain things are possible with a Unit test, which makes sense to skip for e2e.

@aidenwang9867
Copy link
Author

Thanks, @aidenwang9867!
Could you please add unit tests? It adds reliability.
It is critical that we have tests.

Of course! Should I change this to a draft PR for now / add the tests in a follow-up PR? It might take me some extra effort to have the tests covered since I'm not very familiar with testing for an action app.

The tests are standard go tests. Can we add unit tests for this PR? Probably certain things are possible with a Unit test, which makes sense to skip for e2e.

np, will do :-)


func asPointerStr(s string) *string {
return &s
}
Copy link
Author

@aidenwang9867 aidenwang9867 Jul 29, 2022

Choose a reason for hiding this comment

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

Unit tests added for visualization-as-a-comment. Thanks for the suggestion! @naveensrinivasan

@spencerschrock
Copy link
Member

Closing this PR due to age, and also ossf/scorecard#3125

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.

Feature: Give links to dependency packages in the Dependency-diff Visualization markdown result
3 participants