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

✨ show non-compliant code changes for CI-Tests, Code-Review and SAST checks in --show-details mode #2835

Merged
merged 9 commits into from
Apr 21, 2023

Conversation

ashishkurmi
Copy link
Contributor

What kind of change does this PR introduce?

Fix for #2798
This change shows non-compliant code changes for CI-Tests, Code-Review and SAST checks when scorecard is run in --show-details mode.

What is the current behavior?

These checks do not show non-compliant code changes in Details field when scorecard is run in --show-details mode.

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

These shows show non-compliant Pull Requests and/or Commit SHAs in Details field when scorecard is run in --show-details mode.

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #2798

Special notes for your reviewer

Does this PR introduce a user-facing change?

Yes

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

show non-compliant code changes for CI-Tests, Code-Review and SAST checks in --show-details mode

Signed-off-by: Ashish Kurmi akurmi@stepsecurity.io

checks/evaluation/ci_tests.go Outdated Show resolved Hide resolved
checks/evaluation/code_review.go Outdated Show resolved Hide resolved
… checks in --show-details mode

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
ashishkurmi and others added 2 commits April 11, 2023 13:14
Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM. I will let @spencerschrock do a final pass and merge the PR in

Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

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

Overall looks good. Thanks for helping improve the results!

I've left 1-2 comments

checks/evaluation/code_review.go Outdated Show resolved Hide resolved
checks/evaluation/code_review.go Outdated Show resolved Hide resolved
checks/sast.go Outdated Show resolved Hide resolved
checks/sast.go Outdated Show resolved Hide resolved
@raghavkaul
Copy link
Contributor

Wrt. Code Review logging changes, no need to revert them - I'll rebase on top of this.

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
@ashishkurmi
Copy link
Contributor Author

Overall looks good. Thanks for helping improve the results!

I've left 1-2 comments

@spencerschrock thanks Spencer for reviewing the PR and providing your feedback, I have addressed all of your comments in my latest PR.

@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Merging #2835 (d3e19c5) into main (130a31f) will increase coverage by 0.08%.
The diff coverage is 80.48%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2835      +/-   ##
==========================================
+ Coverage   51.64%   51.73%   +0.08%     
==========================================
  Files         158      158              
  Lines       12075    12109      +34     
==========================================
+ Hits         6236     6264      +28     
- Misses       5475     5480       +5     
- Partials      364      365       +1     

Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock spencerschrock temporarily deployed to integration-test April 21, 2023 22:17 — with GitHub Actions Inactive
@spencerschrock spencerschrock merged commit 8db70cf into ossf:main Apr 21, 2023
balteravishay pushed a commit to balteravishay/scorecard that referenced this pull request May 29, 2023
…checks in --show-details mode (ossf#2835)

* showing non-compliant code changes for CI-Tests, Code-Review and SAST checks in --show-details mode

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>

* changing code review non-compliant revision traces to Debug from Warn

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>

* changing ci test non-compliant revision trace to Debug from Warn

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>

* unit test fixes in code_review_test.go

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>

* Incorporating Spencer's feedback

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>

---------

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
Signed-off-by: Avishay <avishay.balter@gmail.com>
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: Add PR information in the details of CI-Tests, Code-Review and SAST checks
4 participants