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

Maintainers policy #1832

Merged
merged 5 commits into from
Feb 9, 2019
Merged

Maintainers policy #1832

merged 5 commits into from
Feb 9, 2019

Conversation

mishmosh
Copy link
Contributor

@mishmosh mishmosh commented Feb 7, 2019

Fixes #516. Not all decisions finalized, but it's time to move the proposal into PR. Comments welcome.

  • Small changes in "Code Reviews" section.
  • Add new "Roles" section.

Fixes #516. Not all decisions finalized, but it's time to move the proposal into PR. Comments welcome. 
* Small changes in "Code Reviews" section.
* Add new "Roles" section.
@anorth
Copy link
Member

anorth commented Feb 7, 2019

I like it, @mishmosh.

One suggestion: in the interests of code security, we might actively remove committers and maintainers who are no longer regularly & actively involved after x months. They're of course still welcome to contribute.

@phritz
Copy link
Contributor

phritz commented Feb 7, 2019

  • we should mention that contributors can get write access but not become a committer once they have proven that they make helpful contributions

@travisperson
Copy link
Contributor

travisperson commented Feb 7, 2019

I did not see this document prior to our meeting. I agree with this document and I think it align pretty well with what we discussed back in January (just with a more open merge model).

I do think we also need to address how we are going to organize these roles inside of go-filecoin, and the filecoin-project organization.

Something worth looking at is Githubs own Open Source Guide, the section on Leadership & Governance I think is worth reading because it looks to match up pretty well with what is proposed here. There is also an interesting section under "How to Contribute" about Anatomy of an open source project.

Question Is the current plan to create Organization Teams to represent these roles? I think it makes sense for the Committers and Maintainers roles. I'd question adding a contributors team (role) just because it's a lot to manage, and I don't believe it provide much value to the organization.

Advantages to creating a Contributor team

  • Users gain an organization badge that they can display under their Github profile.

Disadvantages to creating a Contributor team

  • Something else to manage
  • Opens the Org to accidentally giving org level permissions to almost anyone (because the to join this team, all you have to do is contribute in some way, and ask)

(There might be other advantages / disadvantages, feel free to edit this issue)

Github Social Badges

Something to be aware of: Github itself adds labels along side users depending on how they relate to a given repository, or the organization.

  • Contributor: example This user has previously committed to the repository.
  • Collaborator: example This user has been invited to collaborate on the repository. (This maps directly to the Collaborator section under repo settings)
  • Member: example This user is a member of the organization. (Being part of any team)

There are others, but these are the main ones at an organization level.

Githubs Auth Model

I think most people might be familiar with this, as it's how we manage private-previewers, and a few other things, but I wanted to lay it out here a bit.

Github has three basic levels of authorization, Read, Write, and Admin.

At an organizational level Github provides teams. Teams, if public, can be mentioned through @<org>/<team>. Teams can be given permissions on a per repository basis. Teams can also be constructed under a hierarchy.

Individual repositories can invite Collaborator under Collaborators & teams to provide access locally to just the single repository without having to deal with the greater organization.

@mishmosh
Copy link
Contributor Author

mishmosh commented Feb 7, 2019

via @travisperson:

Question Is the current plan to create Organization Teams to represent these roles? I think it makes sense for the Committers and Maintainers roles. I'd question adding a contributors team (role) just because it's a lot to manage, and I don't believe it provide much value to the organization.

Agreed. I think we should create committers and maintainers teams, but not contributors.

via @phritz:

we should mention that contributors can get write access but not become a committer once they have proven that they make helpful contributions

If we want to create an additional group with permissions somewhere between zero and committer, that should be acknowledged explicitly in the docs and given a specific name. Otherwise, we'll get messy permissions management. Here's a specific straw-person proposal, open to alternatives:

  • Call them Collaborators
  • Give write permissions but block merging to master, which is reserved for committers and maintainers.

@phritz
Copy link
Contributor

phritz commented Feb 7, 2019

that sounds sensible. grant merge to committers and maintainers via https://help.github.com/articles/about-branch-restrictions/, and ensure collaborators cannot.

CONTRIBUTING.md Outdated

Responsibilities:

* TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "follow the community code of conduct" goes here? Also maybe nothing goes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, thanks.

@mishmosh mishmosh changed the title [WIP comments welcome] Maintainers and committers Maintainers policy Feb 8, 2019
Copy link
Contributor

@acruikshank acruikshank left a comment

Choose a reason for hiding this comment

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

LGTM

CONTRIBUTING.md Outdated
`go-filecoin` requires 2 approvals for all PRs, with at least one of {@phritz, @acruikshank, @whyrusleeping}. If your PR hasn't been reviewed in 3 days, pinging reviewers via Github or community chat is also welcome and encouraged.
`go-filecoin` requires 2 approvals for all PRs. We use the following process, which aims to merge code quickly and efficiently while avoiding both accidental and malicious introduction of bugs, unintended consequences, or vulnerabilities.

The first review is done by a committer. Once they deem it ready for merge, they will assign a maintainer for the second review. Once the PR has 2 Approvals and blocking comments have been addressed, the committer rebases and merges the PR.
Copy link
Member

Choose a reason for hiding this comment

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

I would maybe clarify that the committer who approves the PR first should have experience with that part of the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, done.

Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

This is good, I have one comment around clarifying that reviews must come from people with a familiarity with the particular code being reviewed. That could be simplied implied, but i think its cleaner to explicitly say it.

@mishmosh mishmosh merged commit 4e75ee9 into master Feb 9, 2019
@mishmosh mishmosh deleted the maintainers branch February 9, 2019 04:11
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.

None yet

7 participants