-
Notifications
You must be signed in to change notification settings - Fork 603
🌱 New check: Inactive Maintainers #4893
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4893 +/- ##
==========================================
+ Coverage 66.80% 69.68% +2.88%
==========================================
Files 230 257 +27
Lines 16602 17312 +710
==========================================
+ Hits 11091 12064 +973
+ Misses 4808 4233 -575
- Partials 703 1015 +312 🚀 New features to boost your workflow:
|
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
|
This pull request has been marked stale because it has been open for 10 days with no activity |
spencerschrock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some activity types require higher token permissions. If the check runs without those permissions, it will not consider those activity types.
Wouldn't you need an admin token to enumerate the maintainers? So none of the check would run without admin?
I could see this working as a probe in Maintained, which would only be enabled when running with an admin token.
In terms of data collection, did you consider using the existing client methods to look for maintainer activity, instead of just putting them in a new GetMaintainerActivity method?
|
This pull request has been marked stale because it has been open for 10 days with no activity |
What kind of change does this PR introduce?
Fixes #2027.
New feature
This PR adds a new check for inactive maintainers. At a high level, the check does two things in the following order:
The check scores proportionally based on the proportion between active and inactive maintainers.
The client handlers pull in as many activity signals as the API allows. The Gitlab client that Scorecard uses had some bugs with some signal types, so the Gitlab client handler uses raw requests instead of client methods. The GitHub client does not use graphql; I couldn't get it to be as efficient in getting the same details and batching the calls efficiently.
At a bit of a lower level, the client handlers will go through all activity data they have fetched and will mark a maintainer active as soon as they see any activity by that maintainer. Once all maintainers are active, the handlers will not process anymore activity records. This is for efficiency.
The activity data that the GitHub and Gitlab handlers can fetch are documented in the check documentation. The clients pull in a fairly comprehensive set of activity data from the repository. They do not include some GitHub enterprise-only activity types, manual job triggers (GitHub) and Direct PR/MR reviews (Gitlab does not make the timestamp for these available in the API response).
This check should probably be part of the Maintenance check, but before I place it there, I will let folks chime in on thoughts and suggestions.
Some activity types require higher token permissions. If the check runs without those permissions, it will not consider those activity types.
Special notes for your reviewer
This can be tested with:
SCORECARD_DEBUG_MAINTAINERS=1 \ GITHUB_AUTH_TOKEN=$TOKEN \ go run main.go --repo=github.com/owner/repo \ --checks=Inactive-MaintainersSCORECARD_DEBUG_MAINTAINERS=1will print out the active and inactive maintainers.Does this PR introduce a user-facing change?
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.)