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: add TSC as owner of governance-related docs #34737

Closed
wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@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 Aug 11, 2020
@mmarchini
Copy link
Contributor Author

Also @nodejs/community-committee since this PR will add you as owner of some files :)

@jasnell
Copy link
Member

jasnell commented Aug 11, 2020

Likely good to include LICENSE in there as well?

@mmarchini
Copy link
Contributor Author

Right, missed that one

@bnb
Copy link
Contributor

bnb commented Aug 11, 2020

Appreciate you actively including CommComm ❤️

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM

/CODE_OF_CONDUCT.md @nodejs/tsc @nodejs/community-committee
/CONTRIBUTING.md @nodejs/tsc @nodejs/community-committee
/LICENSE @nodejs/tsc @nodejs/community-committee
/GOVERNANCE.md @nodejs/tsc @nodejs/community-committee
Copy link
Contributor

@MylesBorins MylesBorins Aug 12, 2020

Choose a reason for hiding this comment

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

So, historically the CommComm has not been an owner of Governance / Contributing of nodejs/node. I'm not saying that we can't make this change (I've already approved this PR) but I do think this is a fairly large change from what we've done until now... and AFAICT from the charter of the commcomm it is slightly out of scope imho.

I could see how Code of Conduct, License, and potentially Contributing could fall under CommComm but I'm a little less easy about including governance, specifically because this is something the TSC is chartered to manage. CommComm members do not automatically get commit rights for this repo, so it seems a bit out of scope. For the record I would say the same thing about auto adding the TSC to the governance document of any working group.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no objection to @MylesBorins’s comments here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, I'll update :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, kept CommComm on License, Contributing and both Code of Conduct files (yes, we have two)

Copy link
Member

Choose a reason for hiding this comment

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

@MylesBorins ... while that is true, keep in mind that this doesn't change the process for modifying those. It still requires no objections from TSC to modify any part of the governance. There's likely no harm in CommComm being notified of these changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is semantics about if CodeOwners is being used to automate notifications or if it signals ownership / signoff. I was assuming the latter, but my impression seems to be off.

Copy link
Member

Choose a reason for hiding this comment

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

It's ... both? ;-) CODEOWNERS is: these are the people who really need to be aware of these changes. Given that we have a Must Object approach rather than a Must Agree approach, however, with the governance docs it still comes down to TSC objections being most critical. That said, if someone from commcomm raises an objection (or anyone else for that matter), there's going to be an effort made to resolve that before moving forward, regardless of CODEOWNERS

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, if someone from commcomm raises an objection (or anyone else for that matter), there's going to be an effort made to resolve that before moving forward, regardless of CODEOWNERS

Thinking about this a bit more I think what is most important here is that the TSC is onboard with this specific commitment, and since it at least to me appears to be a deviation from current norms that we have broadly made folks aware of this commitment.

What I would not like to see happen is there be an objection in the future that is simply dismissed, or some form of contention that is created by some folks maybe not being ok with this commimtent.

To be clear I'm +1 on making this committment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't expect any objections to be dismissed and would be unpleasantly surprised if it happened either today or after we land this. Collaborators can object to any changes to this repository that target the default branch, in which case: 1) consensus attempt is made via discussions in the PR; 2) objection is escalated to the TSC, who can try to mediate or go to a vote if mediation fails. We don't have exceptions for specific files in the current process.

Either way, CODEOWNERS is not mentioned anywhere in our collaboration docs today, which means it has no clear definition yet, and therefore is only used/considered for notifications.

(just describing what we have today, doesn't mean we can't or should make changes in the future)

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mmarchini mmarchini added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 14, 2020
@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 14, 2020
@github-actions
Copy link
Contributor

Commit Queue failed
- Loading data for nodejs/node/pull/34737
✔  Done loading data for nodejs/node/pull/34737
----------------------------------- PR info ------------------------------------
Title      meta: add TSC as owner of governance-related docs (#34737)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     mmarchini:codeowners-tsc -> nodejs:master
Labels     meta
Commits    1
 - meta: add TSC as owner of governance-related docs
Committers 1
 - Mary Marchini 
PR-URL: https://github.com/nodejs/node/pull/34737
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Myles Borins 
Reviewed-By: Matteo Collina 
Reviewed-By: Gireesh Punathil 
Reviewed-By: Beth Griggs 
Reviewed-By: Shelley Vohr 
Reviewed-By: Trivikram Kamat 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/34737
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Myles Borins 
Reviewed-By: Matteo Collina 
Reviewed-By: Gireesh Punathil 
Reviewed-By: Beth Griggs 
Reviewed-By: Shelley Vohr 
Reviewed-By: Trivikram Kamat 
--------------------------------------------------------------------------------
   ✖  No CI runs detected
   ℹ  This PR was created on Tue, 11 Aug 2020 21:25:20 GMT
   ✔  Approvals: 8
   ✔  - Anna Henningsen (@addaleax) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465441856
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465448967
   ✔  - Myles Borins (@MylesBorins) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465544833
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465634628
   ✔  - Gireesh Punathil (@gireeshpunathil) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465827310
   ✔  - Beth Griggs (@BethGriggs) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465833525
   ✔  - Shelley Vohr (@codebytere) (TSC): https://github.com/nodejs/node/pull/34737#pullrequestreview-465985239
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/34737#pullrequestreview-466491120
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

@MylesBorins MylesBorins added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mmarchini mmarchini added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 14, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 14, 2020
@github-actions
Copy link
Contributor

Landed in 5f78dea

@github-actions github-actions bot closed this Aug 14, 2020
nodejs-github-bot pushed a commit that referenced this pull request Aug 14, 2020
PR-URL: #34737
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 17, 2020
PR-URL: #34737
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@danielleadams danielleadams mentioned this pull request Aug 20, 2020
BethGriggs pushed a commit that referenced this pull request Aug 20, 2020
PR-URL: #34737
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34737
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34737
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.