🌱 Feature: Group commits into changesets #2260

raghavkaul · 2022-09-13T20:32:47Z

What kind of change does this PR introduce?

Introduce a Changeset construct, which contains several Commits that were reviewed as a unit, along with any associated review activity.

PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

Today, scorecard inconsistently uses either Commits or Pull Requests. Commits are more universal than PRs, but evaluating each Commit in a vacuuum may lead to scoring incorrectly when there are multiple, unsquashed commits in a repo's history which all really were part of a singular "code review". This is also the case with CI test runs, where we look at tests run on individual commits, when tests are really run on all commits that are part of an unsquashed PR.

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

Scorecard will batch all related commits that are part of a single GitHub Merge request or Phabricator differential revision into a single Changeset. If a commit is not linked to any external SCM or we can't find the associated code review, the commit is part of a standalone changeset.

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

Special notes for your reviewer

Not all SCMs are supported equally
For Phabricator, we now consider a Differential Revision "reviewed" even if there is no "Reviewed By" line
Scoring should not be affected by this PR

Does this PR introduce a user-facing change?

Raw Results: Commits are now grouped into changesets and associated with review activity, if any

codecov · 2022-09-13T20:38:58Z

Codecov Report

Merging #2260 (f1bb485) into main (3629fd8) will increase coverage by 0.41%.
The diff coverage is 51.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2260      +/-   ##
==========================================
+ Coverage   44.45%   44.87%   +0.41%     
==========================================
  Files          95       95              
  Lines        7987     7953      -34     
==========================================
+ Hits         3551     3569      +18     
+ Misses       4173     4121      -52     
  Partials      263      263

Signed-off-by: Raghav Kaul <[email protected]>

checks/code_review.go

azeemshaikh38 · 2022-09-13T22:56:28Z

I took a quick glance, will take a closer tomorrow and provide my detailed feedback.

Just wanted to comment to say this is an awesome change and will really help improve Scorecard results - thanks for working through this!

clients/pull_request.go

clients/user.go

checks/raw/code_review.go

clients/pull_request.go

Signed-off-by: Raghav Kaul <[email protected]>

* Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]>

Signed-off-by: Raghav Kaul <[email protected]>

github-actions · 2022-09-19T18:16:31Z

Integration tests success for
[ab46601]
(https://github.com/ossf/scorecard/actions/runs/3084340240)

azeemshaikh38

LGTM with comments. Thanks for fixing this!

checker/raw_result.go

checks/code_review_test.go

checks/evaluation/code_review.go

laurentsimon

LGTM

Signed-off-by: Raghav Kaul <[email protected]>

github-actions · 2022-09-20T17:40:42Z

Integration tests success for
[bc07c91]
(https://github.com/ossf/scorecard/actions/runs/3092126985)

github-actions · 2022-09-20T17:53:05Z

Integration tests success for
[f1bb485]
(https://github.com/ossf/scorecard/actions/runs/3092192071)

* Group raw commits into changesets Signed-off-by: Raghav Kaul <[email protected]> * Add tests, fix golint Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> * Fix test failures, remove unneeded fields from raw results Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Fix tests * Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]> * Accept code reviews on any commit, not just HEAD Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Co-authored-by: Azeem Shaikh <[email protected]>

* Group raw commits into changesets Signed-off-by: Raghav Kaul <[email protected]> * Add tests, fix golint Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> * Fix test failures, remove unneeded fields from raw results Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Fix tests * Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]> * Accept code reviews on any commit, not just HEAD Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Co-authored-by: Azeem Shaikh <[email protected]> Signed-off-by: nathaniel.wert <[email protected]>

* Group raw commits into changesets Signed-off-by: Raghav Kaul <[email protected]> * Add tests, fix golint Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> * Fix test failures, remove unneeded fields from raw results Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Fix tests * Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]> * Accept code reviews on any commit, not just HEAD Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Co-authored-by: Azeem Shaikh <[email protected]>

* Group raw commits into changesets Signed-off-by: Raghav Kaul <[email protected]> * Add tests, fix golint Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> * Fix test failures, remove unneeded fields from raw results Signed-off-by: Raghav Kaul <[email protected]> * Fix lint Signed-off-by: Raghav Kaul <[email protected]> * Fix tests * Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]> * Accept code reviews on any commit, not just HEAD Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Co-authored-by: Azeem Shaikh <[email protected]> Signed-off-by: nathaniel.wert <[email protected]>

raghavkaul requested review from azeemshaikh38, justaugustus, laurentsimon and naveensrinivasan as code owners September 13, 2022 20:32

raghavkaul had a problem deploying to integration-test September 13, 2022 20:33 Failure

raghavkaul changed the title ~~Feature changesets~~ 🌱 Feature: Group commits into changesets Sep 13, 2022

raghavkaul added 3 commits September 13, 2022 20:42

Group raw commits into changesets

f1c57dc

Signed-off-by: Raghav Kaul <[email protected]>

Add tests, fix golint

71511b1

Signed-off-by: Raghav Kaul <[email protected]>

Fix lint

4553b84

Signed-off-by: Raghav Kaul <[email protected]>

raghavkaul force-pushed the feature-changesets branch from 5861166 to 4553b84 Compare September 13, 2022 20:46

raghavkaul had a problem deploying to integration-test September 13, 2022 20:47 Failure

laurentsimon reviewed Sep 13, 2022

View reviewed changes

checks/code_review.go Outdated Show resolved Hide resolved

azeemshaikh38 reviewed Sep 14, 2022

View reviewed changes

raghavkaul added 2 commits September 14, 2022 15:53

Address PR comments

6a3bfc8

Signed-off-by: Raghav Kaul <[email protected]>

Fix test failures, remove unneeded fields from raw results

528d6b3

Signed-off-by: Raghav Kaul <[email protected]>

raghavkaul had a problem deploying to integration-test September 14, 2022 18:11 Failure

Fix lint

c517deb

Signed-off-by: Raghav Kaul <[email protected]>

raghavkaul had a problem deploying to integration-test September 14, 2022 18:29 Failure

Merge branch 'main' into feature-changesets

2db2191

raghavkaul had a problem deploying to integration-test September 19, 2022 13:58 Failure

raghavkaul and others added 2 commits September 19, 2022 16:06

Fix tests

c39a948

* Handle randomized order * e2e Signed-off-by: Raghav Kaul <[email protected]>

Merge branch 'main' into feature-changesets

cbfcde8

raghavkaul had a problem deploying to integration-test September 19, 2022 16:11 Failure

Accept code reviews on any commit, not just HEAD

ab46601

Signed-off-by: Raghav Kaul <[email protected]>

raghavkaul temporarily deployed to integration-test September 19, 2022 17:23 Inactive

azeemshaikh38 approved these changes Sep 20, 2022

View reviewed changes

laurentsimon approved these changes Sep 20, 2022

View reviewed changes

Address PR comments

bc07c91

Signed-off-by: Raghav Kaul <[email protected]>

raghavkaul temporarily deployed to integration-test September 20, 2022 17:23 Inactive

Merge branch 'main' into feature-changesets

f1bb485

azeemshaikh38 temporarily deployed to integration-test September 20, 2022 17:34 Inactive

azeemshaikh38 enabled auto-merge (squash) September 20, 2022 17:35

azeemshaikh38 merged commit d75dea8 into ossf:main Sep 20, 2022

raghavkaul mentioned this pull request Sep 27, 2022

Feature: Code Review Check handle commits made by version bump bots #2302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Feature: Group commits into changesets #2260

🌱 Feature: Group commits into changesets #2260

raghavkaul commented Sep 13, 2022 •

edited

Loading

codecov bot commented Sep 13, 2022 •

edited

Loading

azeemshaikh38 commented Sep 13, 2022

github-actions bot commented Sep 19, 2022

azeemshaikh38 left a comment

laurentsimon left a comment

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

🌱 Feature: Group commits into changesets #2260

🌱 Feature: Group commits into changesets #2260

Conversation

raghavkaul commented Sep 13, 2022 • edited Loading

What kind of change does this PR introduce?

What is the current behavior?

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

Special notes for your reviewer

Does this PR introduce a user-facing change?

codecov bot commented Sep 13, 2022 • edited Loading

Codecov Report

azeemshaikh38 commented Sep 13, 2022

github-actions bot commented Sep 19, 2022

azeemshaikh38 left a comment

Choose a reason for hiding this comment

laurentsimon left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

raghavkaul commented Sep 13, 2022 •

edited

Loading

codecov bot commented Sep 13, 2022 •

edited

Loading