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

meta: update comment in CODEOWNERS to better reflect current policy #45944

Merged
merged 3 commits into from
Jan 6, 2023

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Dec 22, 2022

We do not enforce signing off by any code owners when landing PRs.

We may want to take the opportunity to refine our policy regarding CODEOWNERS and what are the criteria to consider a PR ready to land. Currently, here are the criteria listed in the Collaborator Guide:

### Consensus seeking
A pull request can land if it has the needed [approvals](#code-reviews),
[CI](#testing-and-ci), [wait time](#waiting-for-approvals) and no
[outstanding objections](#objections). [Breaking changes](#breaking-changes)
must receive [TSC review](#involving-the-tsc) in addition to other
requirements. If a pull request meets all requirements except the
[wait time](#waiting-for-approvals), please add the
[`author ready`](#author-ready-pull-requests) label.

At least two collaborators must approve a pull request before the pull request
lands. One collaborator approval is enough if the pull request has been open
for more than seven days.
Approving a pull request indicates that the collaborator accepts responsibility
for the change.
Approval must be from collaborators who are not authors of the change.

/cc @GeoffreyBooth @nodejs/tsc

We do not enforce signing off by any code owners when landing PRs.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Dec 22, 2022
@GeoffreyBooth
Copy link
Member

Rather than restate the policy from the collaborator guide, perhaps this comment should just link to it?

Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

LGTM. I agree with @GeoffreyBooth, doc/contributing/collaborator-guide.md is a much better place for this info because all collaborators are familiar with it. I don't think many collaborators will look at comments in a GitHub specific file for our policies.

.github/CODEOWNERS Outdated Show resolved Hide resolved
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@GeoffreyBooth GeoffreyBooth added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 28, 2022
@aduh95
Copy link
Contributor Author

aduh95 commented Dec 28, 2022

I've added a few additional changes which after giving it a few thoughts, I think should belong in this PR:

  • I've removed the ordered list, AFAICT there was no particular order in the list.
  • I've removed the point that says "Each codeowner team should contain at least one @nodejs/tsc member" because I believe we do not enforce this either.

PTAL if you already approved the PR, and consider re-approving if you agree with that new change.

@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 28, 2022
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 1, 2023
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jan 6, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 6, 2023
@nodejs-github-bot nodejs-github-bot merged commit 6f50acd into nodejs:main Jan 6, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 6f50acd

@aduh95 aduh95 deleted the update-codeowners-comment branch January 6, 2023 09:57
RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Jan 17, 2023
We do not enforce signing off by any code owners when landing PRs.

PR-URL: nodejs#45944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jan 20, 2023
We do not enforce signing off by any code owners when landing PRs.

PR-URL: #45944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 20, 2023
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
We do not enforce signing off by any code owners when landing PRs.

PR-URL: #45944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
@juanarbol juanarbol mentioned this pull request Jan 28, 2023
juanarbol pushed a commit that referenced this pull request Jan 31, 2023
We do not enforce signing off by any code owners when landing PRs.

PR-URL: #45944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.