-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
*: create project governance #11175
*: create project governance #11175
Conversation
@gyuho please take a look. If the flow and content looks good to you, I would request others to review. Thanks! |
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.
Thanks @spzala !
GOVERNANCE.md
Outdated
Afterward, create a pull request to remove yourself from the [MAINTAINERS](./MAINTAINERS) | ||
file. | ||
|
||
## Approvers |
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.
Are approvers and reviewers refer to the list in OWNERS file?
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.
@jingyih thank you and yes, that's true. Good idea to provide link to the OWNERS file while mentioning them if that's what you are thinking! Also, most other projects usually have one file - MAINTAINERS or OWNERS - with maintainers/approvers/reviewers listed separately underneath. So I am up for merging them like moving maintainers in the OWNER file or other way.
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 agree, having both files is also confusing to me, which is why I asked the question initially.
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.
Yeah, merging into one sound better.
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.
OK, sounds great!
GOVERNANCE.md
Outdated
reached an impasse with a subset of the community, any contributor may open a GitHub | ||
issue or PR or send an email to `[email protected]`. If the | ||
maintainers themselves cannot decide an issue, the issue will be resolved by a | ||
supermajority of the 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.
s/the maintainers./the maintainers./
?
Create project governance.
Thanks! The new maintainers file looks consistent with the governance doc. |
@spzala Let's highlight this in changelog as well. thx |
lgtm. One discussion: Was there a discussion on the concept of approvers? Have we needed this additional abstraction in the past? And did Fanmin and Anthony agree to be approvers? |
@philips thanks, and good points for discussion something I also thought myself too but I decided to kept the roles the same way except removing overlapping of members between roles. Now that you mentioned, I will reach out to Fanmin and Anthony for their interest. Also, though current roles provides good distinction, most projects only have two (maintainers/approvers and reviewers). I personally is totally good in keeping only two roles (maintainers and approvers/reviewers) if everyone is fine with it. |
In my mind we either:
1. Remove approvers completely.
2. Rename maintainers to approvers.
A reviewer is generally someone who is on the ladder towards maintainership
but isn't experienced/trusted enough to merge code themselves.
IMHO we should just remove approvers because the language of "maintainers"
has been with the project since day 1.
…On Thu, Sep 26, 2019 at 9:09 AM Sahdev Zala ***@***.***> wrote:
lgtm.
One discussion: Was there a discussion on the concept of approvers? Have
we needed this additional abstraction in the past? And did Fanmin and
Anthony agree to be approvers?
@philips <https://github.com/philips> thanks, and good points for
discussion something I also thought myself too but I decided to kept the
roles the same way except removing overlapping of members between roles.
Now that you mentioned, I will reach out to Fanmin and Anthony for their
interest. Also, though current roles provides good distinction, most
projects only have two (maintainers/approvers and reviewers). I personally
is totally good in keeping only two roles (maintainers and
approvers/reviewers) if everyone is fine with it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11175?email_source=notifications&email_token=AAAIGCAD4RVHB4S2JOLOLKDQLTNB7A5CNFSM4IZRZ3CKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7WDUIA#issuecomment-535575072>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAIGCEYGE2HWGOM4SZCSVDQLTNB7ANCNFSM4IZRZ3CA>
.
|
GOVERNANCE.md
Outdated
Contributors who are interested in becoming a maintainer, if performing these | ||
responsibilities, should discuss their interest with the existing maintainers. New | ||
maintainers must be nominated by an existing maintainer and must be elected by a | ||
supermajority of maintainers. Likewise, maintainers can be removed by a supermajority |
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.
extra whitespace character :)
s/supermajority of maintainers/supermajority of 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.
Ahh :), good catch. Thanks @gyuho I will update the 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.
lgtm, thanks for working on this @spzala
LGTM! Thanks! Not sure why travis is complaining about the commit title... |
Thanks @jingyih - interesting, let me update PR to address comment from @gyuho and be more explicit with commit title, however as you pointed out travis failure is strange. |
Create project governance.
LGTM |
Create project governance.