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

Moderation access #469

Closed
wants to merge 3 commits into from
Closed

Conversation

mhdawson
Copy link
Member

No description provided.

@Trott
Copy link
Member

Trott commented Jan 12, 2018

Can you please run this (and all doc contributions) through a spelling/grammar checker?

@mhdawson
Copy link
Member Author

One note is that we are still moving some repos over to nodejs-private, and we should probably wait until that is done to land this (if we reach consensus). I don't think that moving them over should take more than a few days.

* Limiting access to reduce risk
* Enabling individuals to move community work forward without undue delay

When possible we should use automation and tools to reduce the breadth of access
Copy link
Member

Choose a reason for hiding this comment

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

add comma after possible

Copy link
Member

Choose a reason for hiding this comment

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

Probably don't want to use we here or on line 27.

@mhdawson
Copy link
Member Author

mhdawson commented Jan 12, 2018

@Trott do you have a good suggestion for one that you use when generating md files ?

* Enabling individuals to move community work forward without undue delay

When possible we should use automation and tools to reduce the breadth of access
that needs to be provide in order to enable individuals to move community work foward.
Copy link
Member

Choose a reason for hiding this comment

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

typo: be provide -> be provided

Copy link
Member

Choose a reason for hiding this comment

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

forward is misspelled

As these tools are developed and put in place we will reduce the groups to which
org ownership is granted.

At the current time the following groups are granted access in order for them
Copy link
Member

Choose a reason for hiding this comment

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

add a comma after time

to facilitate community work.

* TSC members
* Chair of the Community Comittee
Copy link
Member

Choose a reason for hiding this comment

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

Committee is misspelled

* TSC members
* Chair of the Community Comittee
* Moderation team members. - It is expected that moderation team members will limit
their use of the access granted to that requied to carry out moderation across the
Copy link
Member

Choose a reason for hiding this comment

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

required is misspelled

@Trott
Copy link
Member

Trott commented Jan 12, 2018

@Trott do you have a good suggestion for one that you use when generating md files ?

I usually use the free version of Grammarly, although I don't know how well it does with raw markdown. I have a Markdown editor, so I copy/paste the rendered doc into Grammarly rather than the raw markdown. But I'd guess it's fine, especially for a doc like this. It can generate a lot of false positives, but I think that's pretty much true of all the grammar tools.

@mhdawson
Copy link
Member Author

Pushed commit addressing initial comments.

TSC members and the Chair of the Community Committee are the only individuals granted
Owner permissions within the Node.js GitHub Organization.
As with other resources within the community, Org ownership will be
granted in order to optimize for the conflicting requirements of:
Copy link
Member

@Trott Trott Jan 13, 2018

Choose a reason for hiding this comment

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

We don't use "Org ownership" in the doc. That phrase is pretty colloquial. Stick to the terminology elsewhere: Owner permissions.

"As with other resources within the community..." is filler. Take it out.

So, all told, maybe something like this?:

Whether to grant Owner permissions is determined by optimizing for the following conflicting requirements:

(As an alternative, I'd also be all for taking out all of this explanatory material and just identifying the people that have Owner permissions.)


When possible, automation and tools should be used to reduce the breadth of
access that needs to be provided in order to enable individuals to move
community work forward. As these tools are developed and put in place we will
Copy link
Member

@Trott Trott Jan 13, 2018

Choose a reason for hiding this comment

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

How about this instead?:

As these tools are created, the groups to which Owner permissions are granted will be reduced.

community work forward. As these tools are developed and put in place we will
reduce the groups to which org ownership is granted.

At the current time, the following groups are granted access in order for them
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "At the current time" is superfluous. Take it out.

Copy link
Member

Choose a reason for hiding this comment

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

Also consider removing "in order for them to facilitate community work.". The sentence should end with a colon as well. So all told, maybe this?:

The following groups are granted Ownership permissions:


* TSC members
* Chair of the Community Committee
* Moderation team members. - It is expected that moderation team members
Copy link
Member

Choose a reason for hiding this comment

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

Nit: remove the dash/hyphen/whatever.

Copy link
Member

@Trott Trott Jan 13, 2018

Choose a reason for hiding this comment

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

Nit: Get rid of "It is expected that".

Copy link
Member

Choose a reason for hiding this comment

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

Generally, I think we've been styling it captialized in the docs: Moderation Team

@Trott
Copy link
Member

Trott commented Jan 13, 2018

The TL;DR of this seems to be "Moderation Team gets Owner permissions until there's some other way for them to do their work, which will hopefully be a bot at some point in the not-too-distant future."

+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

@jasnell
Copy link
Member

jasnell commented Jan 16, 2018

Given the objections that @rvagg has raised on this, I'd like for this to to on the tsc-agenda for discussion before it lands. I'm not -1'ing the actual change, I just want it to be discussed on the call.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Temporarily -1'ing to ensure that there is adequate discussion on the tsc call before this lands

@mhdawson
Copy link
Member Author

Was discussed in last TSC meeting, no objections to moving forward. James said we could dismiss his objection.

@mhdawson mhdawson dismissed jasnell’s stale review January 18, 2018 18:27

James agreed we could dismiss in last TSC review.

@jasnell
Copy link
Member

jasnell commented Jan 18, 2018

Also from the TSC meeting: Everyone with owner access needs to have 2FA enabled on their github account. That needs to be confirmed before assigning the owner permission. Also, efforts to pursue automation options need to move forward so that assigning owner permissions to individuals outside the TSC and CommComm chairs no longer becomes necessary. The rationale there is strictly limited to general security concerns -- that is, the higher the number of users with owner permissions, the higher the risk of something bad happening should one of those accounts be compromised. So granting owner permissions to the moderation team should be considered a temporary action until an automated solution can be developed.

@mhdawson
Copy link
Member Author

@Trott took the action to validate 2FA before adding owner access.

@jasnell
Copy link
Member

jasnell commented Jan 18, 2018

took the action to validate 2FA before adding owner access

Yep... for existing mod team members... it needs to be confirmed for new members before assigning permissions.

mhdawson added a commit that referenced this pull request Jan 18, 2018
PR-URL: #469
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: MichaëZasso <targos@protonmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@mhdawson
Copy link
Member Author

Landed as 9cf274c

@mhdawson mhdawson closed this Jan 18, 2018
@mhdawson mhdawson deleted the moderation-access branch August 14, 2019 17:00
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.

7 participants