-
Notifications
You must be signed in to change notification settings - Fork 145
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
Conversation
Initial version of docs for chartering Signed-off-by: Michael Dawson <[email protected]>
@nodejs/package-maintenance first cut at governance needed related to chartering the package maintenance team as a Working Group. |
There was a problem hiding this 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
* 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
||
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Oh, it also occurs to me that we need to align the language to be either |
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 |
There was a problem hiding this comment.
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.
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/) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
I created an "Admins" team in the 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 EDIT: Actually, it would be nice to be able to mention people who work on a given project by the |
Co-Authored-By: Steven <[email protected]>
Co-Authored-By: Steven <[email protected]>
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/) |
There was a problem hiding this comment.
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.
@wesleytodd did you mean the |
@wesleytodd @dominykas @ljharb I think I've addressed the comments/discussion from the last meeting. Please take a look. |
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Dominykas Blyžė <[email protected]>
Co-authored-by: Dominykas Blyžė <[email protected]>
Co-authored-by: Dominykas Blyžė <[email protected]>
Co-authored-by: Dominykas Blyžė <[email protected]>
Co-authored-by: Dominykas Blyžė <[email protected]>
Co-authored-by: Dominykas Blyžė <[email protected]>
Refs: nodejs/admin#470 Refs: nodejs/package-maintenance#338 Signed-off-by: Michael Dawson <[email protected]>
PR to request chartering: nodejs/TSC#880 |
* doc: Charter the package-maintenance working group Refs: nodejs/admin#470 Refs: nodejs/package-maintenance#338 Signed-off-by: Michael Dawson <[email protected]>
Initial version of docs for chartering
Signed-off-by: Michael Dawson [email protected]