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

doc: allow request for TSC reviews via the GitHub UI #56493

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jan 6, 2025

I might be missing some context on why we’re not doing that, but IMO it would make perfect sense for semver-major PRs, as it’s basically the only time where TSC voting members reviews have a different weight. Requesting a review from the team would allow GH UI to highlight those reviews (useful for newcomers who are less likely to have all the TSC members handles in mind). It would require us to give the @nodejs/TSC team Triage role on the repo, which should not change anything as TSC voting members are already collaborators (which have Write role on the repo).

EDIT: from my understanding of #22759, it seems the rule was put in place because of the lack of support for filtering/filtering out notifications coming from team reviews, which was problematic for folks using GitHub professionally and couldn't/wouldn't involve Node.js collaboration during paid hours. It looks like the issue still exists, I have to say it's kind of crazy GitHub doesn't even support filtering out an org from the notification feed – but it does support that when searching for PRs. However, I'd still be open to trying it out again, even if GitHub notification feed filtering hasn't changed much since 2018, the Node.js project has, so it might just work for the current project members.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jan 6, 2025
@cjihrig
Copy link
Contributor

cjihrig commented Jan 6, 2025

I might be missing some context on why we’re not doing that

This is why historically - #22759. I'm not sure if it's still relevant or not.

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 7, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 8, 2025
@nodejs-github-bot nodejs-github-bot merged commit 5747828 into nodejs:main Jan 8, 2025
26 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5747828

@aduh95 aduh95 deleted the request-tsc-reviews branch January 8, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants