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

Initial draft of docs for chartering #338

Merged
merged 31 commits into from
Jul 2, 2020
Merged

Conversation

mhdawson
Copy link
Member

Initial version of docs for chartering

Signed-off-by: Michael Dawson michael_dawson@ca.ibm.com

Initial version of docs for chartering

Signed-off-by: Michael Dawson <michael_dawson@ca.ibm.com>
Governance.md Show resolved Hide resolved
@mhdawson
Copy link
Member Author

@nodejs/package-maintenance first cut at governance needed related to chartering the package maintenance team as a Working Group.

@mhdawson mhdawson changed the title doc: initial draft of docs for chartering WIP doc: initial draft of docs for chartering Apr 17, 2020
Charter.md Outdated Show resolved Hide resolved
Charter.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@wesleytodd wesleytodd left a comment

Choose a reason for hiding this comment

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

Great start! Thanks @mhdawson!

Charter.md Outdated Show resolved Hide resolved
Charter.md Outdated
* Managing the list of organization owners which supplement the standard
Node.js organization owners as outlined in:
[https://github.com/nodejs/admin/blob/master/GITHUB_ORG_MANAGEMENT_POLICY.md#owners](https://github.com/nodejs/admin/blob/master/GITHUB_ORG_MANAGEMENT_POLICY.md#owners)
* Approving new repositories
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this follows the same policy we have of blocking via dissent, otherwise moving forward, would it not be wise to rephrase this to something more like Overseeing repositories (creating, moving, removing)? I feel like the goal is to keep the barriers low to keep productivity high, this wording feels heavy handed to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Works for me

Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated

The package maintenance team policy on landing a PR in the repositories within the Pkgjs
repository is for there to be:
- At least 2 approvals from package-maintenance members
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 we should change this to be 1 approvals from member. The problem right now would be we don't have 2 members watching each of the repos 😀! I also think that we need to add a fast-track policy on here with the same guidelines of 1 approval, where it can circumvent the 72hour hold. Again, with the current small contributor base, it would really slow down the effort to wait 72 hours for each merge.

Copy link
Member

Choose a reason for hiding this comment

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

I personally don't mind the wait period (I've been deliberately keeping my PRs open, even though I'm almost certian nobody will look), although we do need to be able to respond quickly in case of security issues or smth.

Also just to clarify - 1 approval other than the person making the last commit?

Copy link
Member

Choose a reason for hiding this comment

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

I personally don't mind the wait period

I guess I don't really either, just wont want to accidentally add a process which blocks progress. Are you good with the fast-track addition?

Also just to clarify - 1 approval other than the person making the last commit?

Yes! 😀Should we clarify the language?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, having a fast-track for things like security issues or critical bugs does make sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added fast tracking language used in the node core repo.

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'd be ok with 1 approval as well, just started with what's in the Node docs. Do we have concensus that it should be 1?

@wesleytodd
Copy link
Member

Oh, it also occurs to me that we need to align the language to be either Working Group or Team. It uses both in the doc.

the [pkgjs](https://github.com/pkgjs) orgnanization
* Project governance and process (including this policy)
* Managing the maintainer teams and contribution policies for the
following repositories
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a label and rephrase this as "Repositories with pkgjs label under nodejs organization(s)"? Not sure if we need to mention the pkgjs org here again.

Charter.md Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated
##### Maintainers

Maintainers for the repositories in the Pkgjs repository are managed
through the [pkgjs-maintainers](https://github.com/orgs/nodejs/teams/pkgjs-maintainers/)
Copy link
Member

Choose a reason for hiding this comment

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

So that means every maintainer has access to every repo under pkgjs? (not opposed, just looking to clarify)

We probably also need to define who has access to publish to npm.

Do we need to mention 2FA anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

So that means every maintainer has access to every repo under pkgjs?

I think we should create teams for each repo. In the long run if we find this as too much work to maintain, then we can change, but I think in the short term it will be helpful to have fine grained control. Leading into the next topic:

We probably also need to define who has access to publish to npm.

Also, I agree on the publish to npm thing, and was thinking about using a GH action to sync lists (we would use this for Express I think). I can look for an existing one or write one if none is available.

Do we need to mention 2FA anywhere?

Currently I have the setting on the npm org to require it. Is that an available setting in GitHub? If not I think we do need to mention and require it.

Copy link
Member

Choose a reason for hiding this comment

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

yes, a github org can be set to require 2FA for membership (and all should be set to that)

Copy link
Member Author

Choose a reason for hiding this comment

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

@wesleytodd can you write up the text for how you think we should manage?

I agree on the 2FA side.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated above to specify that 2FA will be a requirement for all members.

Copy link
Member

Choose a reason for hiding this comment

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

##### Maintainers

Maintainers for the repositories in the pkgjs organization are managed through teams for each repo. When a new repo is created, the user who requested it is automatically added and given management capability on the repo's team.  The initial committer or any Admin member can add/remove members after an issue is opened on the Package Maintenance repo following the normal review guidelines.  Members of the repo team will also be given publish rights on the registry or any other roles required to ship the library/package/code.

What do you think of something like this? I think this will give us a bit more clarity so that we know everyone with publish rights as well, which I think right now is apparently just me, so that will also need to be fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@wesleytodd did you mean the original requester by this in your suggestion above as opposed to The initial committer ? With that change it looks good to me. Otherwise +1

Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
@wesleytodd
Copy link
Member

wesleytodd commented Apr 19, 2020

I created an "Admins" team in the pkgjs org in anticipation of this landing. It doesn't specify, but do we have a list of initial admins? Will we need to individually propose and approve each person? Or can we just set the current list of active members as the initial admin set? I will also look to see if there is an api to sync the admins team to npm's org level ownership.

For the rest of the repos, I would like to get some publisher automation setup. In the comments @dominykas mentioned this, but would anyone be opposed to me creating teams within pkgjs for each repo like nv-publishers? Then syncing that with npm?

EDIT: Actually, it would be nice to be able to mention people who work on a given project by the @pkgjs/<repo> tag. So how about commit and publish rights just go off @pkgjs/nv?

mhdawson and others added 2 commits April 21, 2020 09:09
Co-Authored-By: Steven <steven@ceriously.com>
Co-Authored-By: Steven <steven@ceriously.com>
Governance.md Outdated
##### Maintainers

Maintainers for the repositories in the Pkgjs repository are managed
through the [pkgjs-maintainers](https://github.com/orgs/nodejs/teams/pkgjs-maintainers/)
Copy link
Member

Choose a reason for hiding this comment

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

##### Maintainers

Maintainers for the repositories in the pkgjs organization are managed through teams for each repo. When a new repo is created, the user who requested it is automatically added and given management capability on the repo's team.  The initial committer or any Admin member can add/remove members after an issue is opened on the Package Maintenance repo following the normal review guidelines.  Members of the repo team will also be given publish rights on the registry or any other roles required to ship the library/package/code.

What do you think of something like this? I think this will give us a bit more clarity so that we know everyone with publish rights as well, which I think right now is apparently just me, so that will also need to be fixed.

@mhdawson
Copy link
Member Author

@wesleytodd did you mean the original requester by this in your suggestion above as opposed to The initial committer ? With that change it looks good to me.

Governance.md Outdated Show resolved Hide resolved
@mhdawson
Copy link
Member Author

mhdawson commented May 6, 2020

@wesleytodd @dominykas @ljharb I think I've addressed the comments/discussion from the last meeting. Please take a look.

README.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
All repositories under the management of the package maintenance team must include:

* LICENCE.md from the list approved by the OpenJS Foundation
* CODE_OF_CONDUCT.md referencing the Node.js Code of conduct in the admin repo.
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to adjust the wording to cover the fact that this will be in the .github repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

There was some discussion in the OpenJS Cross project council that at least for the CODE_OF_CONDUCT.md we may require a copy or link in the repo, otherwise it's not discoverable since you don't see it when looking in or cloning th erepo.

Copy link
Member

Choose a reason for hiding this comment

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

Would a link in the README suffice?

That said, there's something comforting about that file just being present at the root of the repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was the sentiment, it's most easily discover-able and clear that there is one if the file is there at the root of the project. I think it could be a link to the shared one so that after it's committed it never needs an update.

Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
Governance.md Outdated Show resolved Hide resolved
mhdawson and others added 6 commits May 25, 2020 10:33
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
Co-authored-by: Dominykas Blyžė <hello@dominykas.com>
@mhdawson mhdawson changed the title WIP doc: initial draft of docs for chartering Initial draft of docs for chartering Jun 9, 2020
mhdawson added a commit to mhdawson/TSC that referenced this pull request Jun 9, 2020
Refs: nodejs/admin#470
Refs: nodejs/package-maintenance#338

Signed-off-by: Michael Dawson <michael_dawson@ca.ibm.com>
@mhdawson
Copy link
Member Author

mhdawson commented Jun 9, 2020

PR to request chartering: nodejs/TSC#880

@ghinks ghinks merged commit 60e8657 into nodejs:master Jul 2, 2020
mhdawson added a commit to nodejs/TSC that referenced this pull request Jul 2, 2020
* doc: Charter the package-maintenance working group

Refs: nodejs/admin#470
Refs: nodejs/package-maintenance#338

Signed-off-by: Michael Dawson <michael_dawson@ca.ibm.com>
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.

8 participants