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

🌱 Migrate Maintained check to probes #3507

Merged
merged 22 commits into from
Nov 17, 2023

Conversation

AdamKorcz
Copy link
Contributor

What kind of change does this PR introduce?

Feature.
(Is it a bug fix, feature, docs update, something else?)

What is the current behavior?

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

This rewrites the Maintained check to four different probes:

  1. archived: Checks whether a project is archived.
  2. activityOnIssuesByCollaboratorsMembersOrOwnersInLast90Days: Checks whether project collaborators, members or owners have participated in issues in the last 90 days.
  3. commitsInLast90Days: Checks whether the project has had any commits by any user type in the last 90 days.
  4. wasCreatedInLast90Days: Checks whether the project was created in the last 90 days.

Put together, these four probes do the same checks as the current Maintained check. There should not be change in scoring, but the unit tests for the Maintained check and evaluation was not extensive, so this requires a review.

A few good-to-knows about the probes:
The outcome of archived may confuse some users: If the project is archived, the outcome is negative, because it is a negative factor. Some users may expect that a negative outcome means "no: The project is not archived".

I believe commitsInLast90Days and activityOnIssuesByCollaboratorsMembersOrOwnersInLast90Days are the first probes to return a numeric value instead of true/false. However, there are no changes to the internals of Scorecard, since each probe is already designed to return a slice instead of a single finding.

The probes are named according to the time they check.

The current Maintained check is not completely aligned with the documentation. For example, the documentation specifies that Maintained checks for commits every week whereas this is not the case in the implementation. The implementation checks that the contributions on average equal one contribution per week. This PR does NOT fix that, and I argue this PR should not fix that, since there are some considerations to make about the scoring.

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

Does this PR introduce a user-facing change?

No

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


@AdamKorcz AdamKorcz temporarily deployed to gitlab September 22, 2023 15:30 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test September 22, 2023 15:30 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #3507 (25b079d) into main (be0b915) will decrease coverage by 5.54%.
The diff coverage is 79.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3507      +/-   ##
==========================================
- Coverage   76.13%   70.59%   -5.54%     
==========================================
  Files         205      209       +4     
  Lines       14068    14217     +149     
==========================================
- Hits        10711    10037     -674     
- Misses       2725     3592     +867     
+ Partials      632      588      -44     

checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/maintained.go Outdated Show resolved Hide resolved
probes/archived/def.yml Outdated Show resolved Hide resolved
probes/commitsInLast90Days/def.yml Outdated Show resolved Hide resolved
probes/wasCreatedInLast90Days/def.yml Outdated Show resolved Hide resolved
@github-actions
Copy link

Stale pull request message

@AdamKorcz AdamKorcz requested a review from a team as a code owner October 31, 2023 11:46
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 31, 2023 11:46 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 31, 2023 11:47 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 31, 2023 11:56 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 31, 2023 11:57 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 31, 2023 12:44 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 31, 2023 12:44 — with GitHub Actions Inactive
Copy link
Contributor

@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.

main blocker is the finding repetition vs value fields. happy to discuss in slack as well

checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/evaluation/maintained.go Outdated Show resolved Hide resolved
checks/evaluation/maintained.go Show resolved Hide resolved
probes/commitsInLast90Days/def.yml Outdated Show resolved Hide resolved
probes/commitsInLast90Days/impl.go Outdated Show resolved Hide resolved
probes/issueActivityByProjectMember/impl.go Outdated Show resolved Hide resolved
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
…st90Days' to 'issueActivityByProjectMember'

Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
@spencerschrock spencerschrock merged commit 1c3d9eb into ossf:main Nov 17, 2023
38 checks passed
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.

3 participants