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: update GitHub Org Management Policy to reflect actual practice #456

Merged
merged 12 commits into from
Jan 11, 2018

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 27, 2017

There are several aspects of the GitHub org management policy that are
not followed in practice and may be unworkable. This change tries to
move the document closer to actual practice.

@nodejs/moderation @nodejs/community-committee @nodejs/tsc

Note that the intention here isn't to say that we can't or shouldn't do the things that are in the policy currently, but since we don't and since it's clear from nodejs/admin#33 that we aren't going to do so imminently, let's update the policy to be more accurate, and then discuss any changes that people want.

I know this will be disappointing and frustrating to many people, but I also think it's better to have an accurate document for us to discuss than to have it be de facto acceptable that we simply ignore this document.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Generally 👍 to this, thank you for picking it up!


Upon the completion of the terms of service for each of these individuals,
their Owner permissions within the GitHub organization shall be removed.
The TSC Director shall be the only individuals granted Owner permissions within
Copy link
Member

Choose a reason for hiding this comment

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

s/TSC Director/TSC members/ (for current practice) or s/individuals/individual/?

Copy link

Choose a reason for hiding this comment

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

oh, i wasn't sure whether this actually only meant TSC director, if it means the TSC members then my review is redundant

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks.

TSC oversight.

Repositories that are under the operational direction of the Community Committee
are subject to CommComm oversight.
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 not quite obvious to me why this is being removed … because we don’t currently follow it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sort of. Also, it's redundant (or maybe just has some nuance I'm missing?). It basically can be paraphrased as: "Repositories that are under CommComm oversight are subject to CommComm oversight." Like, I read that, and other than including CommComm, I'm not sure why it's even there.

Copy link
Member

Choose a reason for hiding this comment

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

I agree its bad to have a doc that does not reflect reality but I think instead of removing references to the Community Committee having oversight as the interim solution, we should be making the CommComm chairs owners in addition to all TSE members. I don't think its our final state but I think its a more acceptable interim state than simply removing any CommComm onership.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhdawson If you would like to add a commit to make that change, feel free. We will need everyone who has approved to re-approve, I think (or remove their approval if they don't approve). My intention had been to open a PR to that effect immediately after this lands, but if you think it's better to do that now, I'm certainly OK with seeing what everyone thinks about that.


Upon the completion of the terms of service for each of these individuals,
their Owner permissions within the GitHub organization shall be removed.
The TSC Director shall be the only individuals granted Owner permissions within
Copy link

Choose a reason for hiding this comment

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

individuals should be singular

Copy link
Member Author

@Trott Trott Dec 27, 2017

Choose a reason for hiding this comment

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

Thanks. I removed Director. (Missed it the first time.) Should say: The TSC shall be the only individuals granted Owner permissions....

(Note that I'm not saying this is the way it should be but it is the way it is and there's at least some resistance to changing it based on conversation in nodejs/admin#33.)

@rvagg
Copy link
Member

rvagg commented Jan 2, 2018

I'm not a fan of this doc in general but this is an improvement so +1 from me

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 with some nits


Upon the completion of the terms of service for each of these individuals,
their Owner permissions within the GitHub organization shall be removed.
The TSC shall be the only individuals granted Owner permissions within the
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be “The TSC members”.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed.

No repository may be deleted, transferred in to, or transferred out of, the
Node.js Foundation GitHub Organization without a simple majority of both
the TSC and CommComm in *favor* of the action. In certain cases, Node.js
Foundation Board of Directors approval may also be required.
Copy link
Member

Choose a reason for hiding this comment

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

I think this paragraph is still needed, without the CommComm reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reinstated. Thanks.

@mhdawson
Copy link
Member

mhdawson commented Jan 2, 2018

Pushed commit with my suggested changes after discussing with @Trott .

@mcollina @fhinkel @cjihrig can you re-review as my commit changes enough to warrant you reviewing again and either confirming you are ok with it or revoking your ok.

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

@mhdawson
Copy link
Member

mhdawson commented Jan 2, 2018

@rvagg see you had +1 instead of approval with button so FYI to you to re-review as well.


Upon the completion of the terms of service for each of these individuals,
their Owner permissions within the GitHub organization shall be removed.
TSC members and the Chair of the Community Comittee are the only individuals granted
Copy link
Member Author

Choose a reason for hiding this comment

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

Committee is misspelled here.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Actually, never mind, I'll correct the spelling. No need to make someone else do it when I can just use GitHub's edit feature and be done with it.)

@cjihrig
Copy link
Contributor

cjihrig commented Jan 2, 2018

I'm still OK with this after @mhdawson's commit.


### Owners

TSC members and the Chair of the Community Committee are the only individuals granted
Copy link

Choose a reason for hiding this comment

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

there can be multiple chairs of the commcomm

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, I thought that was a detail we could leave out with the the assumption it would include chairs in the exception case (which of course is what we have now).

Copy link

Choose a reason for hiding this comment

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

that works!

The [Node.js admin repository](https://github.com/nodejs/admin) will serve as
the central location for managing Node.js GitHub Organization administrative
activities. Only Node.js GitHub Organization owners, TSC members, and Community
Committee members will have write permissions to the Node.js admin repository.
Copy link
Member

Choose a reason for hiding this comment

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

s/will have/have/?

### Members

GitHub users are added as members to the Node.js GitHub Organization when they
are added to any Working Group. Organization Owners should add new members
Copy link
Member

Choose a reason for hiding this comment

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

At the moment we also add users when they are added to a team (not necessarily belong to any charted WG), so maybe ...to any Working Groups or teams.?


GitHub users are added as members to the Node.js GitHub Organization when they
are added to any Working Group. Organization Owners should add new members
to the organization when requested by a Working Group.
Copy link
Member

@joyeecheung joyeecheung Jan 4, 2018

Choose a reason for hiding this comment

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

...by a Working Group or a team.

Also it may be worth mentioning the owner should suggest new members to follow https://github.com/nodejs/admin/blob/master/CODE_OF_CONDUCT.md and enable 2FA prior to adding them to the organization. I believe we usually do that when adding people.


No repository may be deleted, transferred in to, or transferred out of the
Node.js Foundation GitHub Organization without a simple majority of both the
TSC and CommCom in favor of the action. In certain cases, Node.js Foundation
Copy link
Member

Choose a reason for hiding this comment

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

s/CommCom/CommComm


## Repositories

Any organization member may request the creation of a new repository within the
Copy link
Member

Choose a reason for hiding this comment

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

Maybe moving how to do this to the first sentence to increase visibility, e.g. ...by opening an issue in the Node.js admin repository.?

be approved by the TSC and CommComm and are subject to regular security audits.
Bots that perform actions on behalf of the project (such as moderation or membership
management actions) are required to maintain a log, accessible to all individuals
granted Owner permissions, of all actions taken.
Copy link
Member

@joyeecheung joyeecheung Jan 4, 2018

Choose a reason for hiding this comment

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

In the current practice, only people who have added their public keys to the related directory in the secrets repo have access. I would consider documenting this as .. required to maintain a log of all actions taken. The secrets and the logs of the bots and the services are accessible to individuals granted permissions in the nodejs/secrets repo.

Also it might be worth mentioning https://github.com/nodejs/automation/blob/master/enable-travis-under-nodejs.md here, maybe something like See [this document][] on how to migrate Travis integration of a repository transferred into the Node.js organization.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I'll get this right. Maybe you or someone else can propose this particular change subsequently? (Or add a squash commit to this PR if you prefer?)

Copy link
Member

Choose a reason for hiding this comment

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

@Trott Sure, I can open a subsequent PR for this.

a project under the ownership of the Node.js Foundation, and thereby subject
to the Intellectual Property and Governance policies of the Foundation.

No repository may be deleted, transferred in to, or transferred out of the
Copy link
Member

Choose a reason for hiding this comment

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

s/in to/into/

@bnb
Copy link
Contributor

bnb commented Jan 8, 2018

@Trott Could you please add cc-agenda label to this issue? 🙏

@Trott
Copy link
Member Author

Trott commented Jan 10, 2018

Per TSC meeting discussion, this is ready to land but we're going to leave it open for another 48 hours or so. If anyone has objections, now is the time to say so.

@ghost
Copy link

ghost commented Jan 10, 2018

@Trott let's wait until after tomorrow's CC meeting

@ghost
Copy link

ghost commented Jan 11, 2018

nobody from the commcomm objected to this, so i'd say it's ready to land @Trott

@hackygolucky
Copy link
Contributor

hackygolucky commented Jan 11, 2018

I'm +1 to shipping this from the CommComm and Moderation team side if we can immediately start working on a doc that enables both groups to do the work they need to do by getting appropriate permissions to the org. I understand the need for accurate reflection of the-world-as-it-exists.

We've had this discussion about org ownership and how it needs to evolve in multiple issues, PRs, and in TSC/CommComm meetings over the last year, with little movement.

I think we need a coordinated discussion with both TSC, CommComm, and Moderation members to make sure we've documented the needs so that we can expedite this instead of running in conversation circles and finding folks not able to do their work. For ex: a bot is suggested most times these convos come up, but there's work required for that to happen and no one with bandwidth to do that work, which makes it a great future solution but not feasible right now or in the near term.

Also something brought up in CommComm convo: Board member access as org owners. As far as I'm aware, the Board, as representatives of the fiduciary responsibility of the Foundation, have full access to all properties of the Foundation, including the project. Meaning, I don't think the Board directors necessarily need/want(you'd want to ask them) org owner responsibilities, but legally can see anything in the project including private repos if they were to request it. They are held to a high legal standard for privacy and protecting the Node.js Foundation entity as directors, so them abusing that access is something they would get in big trouble for if it were to occur(so it puts the fear into them to not want to).

There are several aspects of the GitHub org management policy that are
not followed in practice and may be unworkable. This change tries to
move the document closer to actual practice.
@Trott
Copy link
Member Author

Trott commented Jan 11, 2018

Now that this has landed, it is official policy that CommComm chairs have owner permissions, so I'm going to add them. (When is the next election for CommComm chair?)

@ghost
Copy link

ghost commented Jan 11, 2018

@Trott september 2018

@ghost
Copy link

ghost commented Jan 11, 2018

@hackygolucky do you want to create an issue that tries to pull all of the future and past discussion about this together?

@bnb bnb removed the cc-agenda label Feb 16, 2018
@Trott Trott deleted the GitHub branch March 11, 2021 14:35
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.