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: improve definition of a collaborator #14981

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 22, 2017

Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators
@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Aug 22, 2017
@benjamingr
Copy link
Member

Maybe we should change the phrasing on the home page to 'core collaborators' to prevent confusion?

@jasnell
Copy link
Member Author

jasnell commented Aug 22, 2017

@benjamingr ... that's fine but can be done in a separate PR

Trott
Trott previously requested changes Aug 22, 2017
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

I support the spirit of this, but this is in contradiction with (and is unworkable with) a large amount of our current governance policies etc. We'd need to change a lot of other things (which we can totally do!) before this can land. A few that come just off the top of my head:

  • Collaborators are listed in the README. This definition will expand that list by hundreds of people. It will remove clarity about who can land PRs in core.

  • @-nodejs/collaborators no longer maps to the complete list of collaborators.

  • The bulk of the COLLABORATORS_GUIDE is written with the assumption that the individual has access to start CI jobs, land code in the core repo, etc.

@Trott Trott dismissed their stale review August 22, 2017 18:25

Oh, hey, looks like this has already kinda-sorta been addressed.

@addaleax
Copy link
Member

As I understood it, this would still keep the individual collaborator groups separate, just point out that the term “Collaborator” refers to any collaborator on repositories within the organization?

@Trott
Copy link
Member

Trott commented Aug 22, 2017

Partial list of things that will need to happen if this lands:

  • We'll need to update "Collaborator" to "Core Collaborator" in a lot of places: the README, the COLLABORATORS_GUIDE, probably some other parts of this very GOVERNANCE doc...

  • We'll need to add a definition for "Core Collaborator". (Could be tricky to keep it from implying a hierarchy, if that's what we're trying to avoid with this change.)

  • We'll need to update the name of the nodejs/collaborators team.

  • We'll have to change the name of the COLLABORATORS_GUIDE.

@Trott
Copy link
Member

Trott commented Aug 22, 2017

As I understood it, this would still keep the individual collaborator groups separate, just point out that the term “Collaborator” refers to any collaborator on repositories within the organization?

@addaleax This document does not use the term that way, even after this change. The way it's being defined in this change, members of the Website WG or Build WG are Collaborators. Which: Cool! But the immediate next sentence of the doc says that Collaborators are selected by the CTC. Those folks are not selected by the CTC.

GOVERNANCE.md Outdated
@@ -8,10 +8,17 @@ Committee (CTC) which is responsible for high-level guidance of the project.
The [nodejs/node](https://github.com/nodejs/node) GitHub repository is
maintained by Collaborators who are added by the CTC on an ongoing basis.

A Collaborator is defined as any individual having commit access to any
Copy link
Member

Choose a reason for hiding this comment

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

This is a great clear statement, but it is in apparent contradiction with both the preceding sentence and the immediately following sentence. I'm a fan of trying to use terminology in an inclusive way, but this doc needs to be treated with considerable care.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

I like the additional text in the paragraph explaining what should be considered for Collaborator nominations. The re-definition of the term Collaborator is well-intentioned but problematic from a practical point of view IMO. Might we split that out into a separate PR? The additional text in the second later paragraph is not dependent on the definition change and we could land that relatively quickly.

@jasnell
Copy link
Member Author

jasnell commented Aug 22, 2017

@Trott, feel free to push commits to the branch with the specific language you'd prefer. I trust you implicitly on your judgement there so there's no reason to seek my approval to land suggested changes.

@Trott
Copy link
Member

Trott commented Aug 22, 2017

@Trott, feel free to push commits to the branch with the specific language you'd prefer. I trust you implicitly on your judgement there so there's no reason to seek my approval to land suggested changes.

@jasnell and everyone who has already approved: I did a pretty drastic overhaul. I left it as a separate commit so it could be force-pushed out if you disagree with the changes. PTAL.

@jasnell
Copy link
Member Author

jasnell commented Aug 22, 2017

As I said, I trust you implicitly. The changes LGTM

@Trott
Copy link
Member

Trott commented Aug 22, 2017

As I understood it, this would still keep the individual collaborator groups separate, just point out that the term “Collaborator” refers to any collaborator on repositories within the organization?

I see a couple of options. One is we could put something like this in this doc:

A Node.js Collaborator is any individual having commit access to any repository in the Node.js GitHub organization. In this document and elsewhere in the nodejs/node repository, Collaborator refers specifically to people who have commit access to the nodejs/node repository.

Another option is that we could put the definition of Node.js Collaborator in a different doc. I'd suggest a doc in the TSC repo but one disadvantage to that is that it may have less visibility. (Then again, I'm not sure the GOVERNANCE doc gets a whole lot of eyeballs either, TBH.)

@@ -9,9 +9,21 @@ The [nodejs/node](https://github.com/nodejs/node) GitHub repository is
maintained by Collaborators who are added by the CTC on an ongoing basis.

Individuals identified by the CTC as making significant and valuable
Copy link
Member

@gibfahn gibfahn Aug 22, 2017

Choose a reason for hiding this comment

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

So all collaborators are approved by the CTC? Assuming collaborators will now roughly map to @nodejs/members, does this mean everyone who joins the org needs CTC approval? Even if they're joining CommComm for example?

Also is this the current process? If not it seems like something that could use more general members feedback.

The answer could totally be "this is a hard question, let's roll with this and improve it going forward".

EDIT: I think this is what @Trott said:

But the immediate next sentence of the doc says that Collaborators are selected by the CTC. Those folks are not selected by the CTC.

Copy link
Member Author

Choose a reason for hiding this comment

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

"approval" does not necessarily mean implicit voting.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but identified suggests it's an active thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the original language and it is definitely lacking. Specific suggestions for improvement are welcome as I'm not sure what would be better

* contributions to the Node.js website
* assistance provided to end users and novice contributors
* participation in Working Groups
* other participation in the wider Node.js community
Copy link
Member

@TimothyGu TimothyGu Aug 23, 2017

Choose a reason for hiding this comment

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

I believe it's important to bring active members of the Node.js community into the project somehow, but at this point I'm not convinced a contributor without much experience in working on the nodejs/node repo should be given commit access to it (following content is added after edit) for the same reason that I, a Collaborator but not member of any WG, am not given commit access to any WGs' repos.

Copy link
Member

@gibfahn gibfahn Aug 24, 2017

Choose a reason for hiding this comment

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

AIUI "commit access to the project" does not mean "commit access to nodejs/node", it means commit access to whichever team you get added to. So if you contribute to core you get made a core collaborator and added to @nodejs/core (assuming we change @nodejs/collaborators to that). If you contribute to the website, then you become a website collaborator and get website access.

To clarify, maybe something like:

-commit access to the project. Activities taken into consideration include (but
+commit access to that repository. Activities taken into consideration include (but

Copy link
Member Author

Choose a reason for hiding this comment

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

We seriously need to get better at defining and clarifying the differences :-( ... going to stew on it more

@hackygolucky
Copy link
Contributor

hackygolucky commented Aug 24, 2017

So the idea here, moving forward, is that once this gets merged, the nodejs/moderation team would be the specifically chosen group of moderators who could be @-ed as a group whenever -anyone-
who is in our GitHub community needs to request moderation, and outside of GitHub would send a request to reports@nodejs.org(which also goes to the members of nodejs/moderation). Then, the Moderation team is made fully aware that they will need to act when these flags are raised, and how to act.

The Moderation repo would then be used for moderation administration for the Moderation team, not for reporting, because that is not accessible to the entire community.

We would make sure that these instructions are IN our CoC and moderation guidelines, so anyone visiting our community knows how to flag a moderator, regardless of whether they are a collaborator.

edited: (I'd be happy to held execute on said above work)

jasnell added a commit that referenced this pull request Aug 24, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: #14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Aug 24, 2017

Landed in 52fe762...

I plan to take another iteration on this shortly

@jasnell jasnell closed this Aug 24, 2017
addaleax pushed a commit to addaleax/ayo that referenced this pull request Aug 25, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: nodejs/node#14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Aug 28, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: nodejs/node#14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: #14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: #14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 20, 2017
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: #14981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
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.

None yet