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

Restrict cq permissions #33

Closed
wants to merge 2 commits into from
Closed

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 13, 2021

No description provided.

Copy link
Contributor

@mmarchini mmarchini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • there's a lot of commits that shouldn't be included here
  • back when we were discussing the commit-queue, it was agreed that the triage team should be allowed to start commit queue as well. This PR would revert that decision, so if that's something we're considering moving forward with we might want to bring it up on a TSC meeting

Was this PR inspired by a recent misuse of the commit queue, or is it a preventive measure to limit CQ access?

@aduh95
Copy link
Contributor Author

aduh95 commented Jun 14, 2021

  • there's a lot of commits that shouldn't be included here

Yep those commits come from nodejs/node, it looks like this repo is a few commits behind.

  • back when we were discussing the commit-queue, it was agreed that the triage team should be allowed to start commit queue as well.

I don't think this is documented anywhere though, and it seems many folks don't know about that.

Was this PR inspired by a recent misuse of the commit queue, or is it a preventive measure to limit CQ access?

It was inspired by a recent email conversation with TSC members, some folks have expressed that it was likely best to restrict the CQ to onboarded collaborators – currently triagers and members of the moderation team can label PRs. No misuse to report.

FWIW I personally like the idea of triagers being able to use the CQ, I'm definitely +1 on having a public discussion to reach a consensus.

@mmarchini
Copy link
Contributor

mmarchini commented Jun 14, 2021

Oops, since it was under a thread on a different subject I missed that this was inspired by a discussion in the private TSC mailing list. This discussion should be had in public since the prior decision was reached in public (see
nodejs/admin#552 and
nodejs/TSC#907 (comment) and
nodejs/TSC#907 (comment) and
nodejs/node#34112 (comment) and https://github.com/nodejs/TSC/blob/main/meetings/2020-08-13.md).

I also missed that this was the auto test repository, feel free to force push to master if needed.

@targos
Copy link
Member

targos commented Jun 14, 2021

There's a difference between being able to do something and being allowed. I don't think non-collaborators should be allowed to start the commit queue, but this can be discussed somewhere else.

@mmarchini
Copy link
Contributor

@targos note that the question posed to TSC on nodejs/TSC#907 (comment) was "should the triage team be allowed to start Commit Queue" (allowed, not able). Besides, I just went through our collaborator guide and couldn't find anything saying that the collaborators team is the only team allowed to land pull requests today. I don't think it hurts for us to discuss, decide and formalize who is allowed to land pull requests. I can open an issue on TSC repo.

@mmarchini
Copy link
Contributor

There we go: nodejs/TSC#1039

@panva panva force-pushed the main branch 29 times, most recently from 6fb17d1 to c8e2f3f Compare February 7, 2023 09:21
@panva panva closed this Feb 7, 2023
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.

4 participants