-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat(meta): require collaborators to be active #7775
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #7775 +/- ##
==========================================
+ Coverage 75.31% 75.33% +0.01%
==========================================
Files 96 96
Lines 7856 7856
Branches 192 192
==========================================
+ Hits 5917 5918 +1
+ Misses 1938 1937 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Is this too many unit tests? I may have gotten carried away...
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.
Pull Request Overview
Adds automation to identify and report inactive collaborators, updates documentation to clarify collaborator maintenance, and schedules a periodic workflow.
- Introduces a script that searches for collaborator activity over a configurable timeframe and files or updates an issue listing inactive members
- Provides comprehensive tests for all new utilities and workflows
- Extends CONTRIBUTING.md with a “Maintaining Collaborator Status” section and adds a GitHub Actions workflow to run the script weekly
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
apps/site/scripts/find-inactive-collaborators/index.mjs | New script to detect inactive collaborators and create/update an issue |
apps/site/scripts/find-inactive-collaborators/tests/index.test.mjs | Tests covering date utilities, activity checks, issue creation/update, and full workflow |
CONTRIBUTING.md | Added “Maintaining Collaborator Status” section and fixed link case |
.github/workflows/find-inactive-collaborators.yml | Scheduled GitHub Actions workflow to invoke the script weekly |
Comments suppressed due to low confidence (1)
CONTRIBUTING.md:6
- There’s an extra hyphen and inconsistent indentation before the 'Becoming a collaborator' entry, which breaks the list structure; remove the redundant '-' to fix formatting.
- - [Becoming a collaborator]
Co-authored-by: Copilot <[email protected]> Signed-off-by: Aviv Keller <[email protected]>
This feels very odd to put inside the site app when it is an entirely meta thing to the repo/organisation, nothing to do with the production site? |
I just figured I'd put it in the same directory as lighthouse, but I can move it. |
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.
maybe put ~/scripts
instead of ~/.github/scripts
not to hide this script
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.
Why? .github/scripts
makes sense, since this is only used by GitHub Actions
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.
I'm fine with .github/scripts, but this format I've never seen before. I'd rather embedding a minimal script directly on the YAML, or publish this script as an Action step itself :)
I agree. But at the same time, there's no such... hmm.. meta repo for the website team or web-infra team, so no idea where this should live. @bmuenzenmeyer we could create a repo on nodejs/web-team as a meta space for the website and web-infra teams. |
WDYT about storing a list of the current members in the GOVERNANCE file? That would eliminate the need to use a custom token, and we could just use the default actions token? (This is what is done in core) |
Fwiw, I'm fine with this living in .github/scripts -- I think this repo is still canonically the right place, and that directory seems like a logical location to me 👍 |
This does seem easier than needing a custom token. I do wonder if something should be done more centrally to have all the (public) teams in code somewhere. |
We have a monorepo now. It would be easy to make a module here that only serves as a tooling/scripts location. Heck, api-docs-tooling could even migrate. |
I don't think the website repo should be a "fit everything" in one repository. There should be separation of concerns IMO. And I do prefer having a meta-repo for the administrative side of things of the web teams. |
Fixes #7767
The wording can be changed as needed.
Requires a
READ_ONLY_PUBLIC_REPO_TOKEN
with:public_repo
(for opening the issue)read:org
(for checking team membership)