-
Notifications
You must be signed in to change notification settings - Fork 66
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 maintainers and governance framework #31
base: master
Are you sure you want to change the base?
Conversation
diskwarrior
commented
Jan 16, 2021
•
edited
Loading
edited
- define expectations for PR reviews
- add process for Issues
- flush out governance definitions
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.
Comments are inline in the review.
|
||
|Maintainer|Role| | ||
|---|---| | ||
|*Dan Harris|CSC Operations| |
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.
We may want to keep the roles aligned with recognized Center categories (groups, teams, etc.). So maybe CSC Operations is ACO (group) or User Operations (team); CSC Applications would be CSSO (group) or User and Applications Support (team). I like the team labels, as it raises visibility of the things we do within the Center and the groups.
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.
+1 on teams
@@ -0,0 +1,79 @@ | |||
# Maintainer Guidelines | |||
|
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.
We might benefit from a sense of the Git-fu a maintainer should have. For those who aren't able to develop that level, is there a different workflow than the more advanced folks? Maybe a "Content Maintainer" would work through the Github interface only on existing material via the editing interface and the master branch, and a "Content Creator" would have more expectations around PRs, reviews, creation, branches, merging, etc.
Reading further, I think I see now the intent. I'm a little concerned that the iterative PR/review process is heavyweight enough to put some folks off of contributing. What might a process be for someone who has ideas for improvement, but doesn't have a Git background?
|
||
It is every maintainer's responsibility to: | ||
|
||
* Expose a clear roadmap for improving the repository. |
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.
"Expose" is a little vague. I think Lex may be the only project I've seen at NREL with a clear roadmap, and even that might be debated by those who were doing the work. Not sure what this bullet means.
|
||
### I'm a maintainer, should I make pull requests too? | ||
|
||
Yes. Nobody should ever push to master directly. All changes should be |
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 the various dev branches expected to go away once this repo reaches a certain maturity? If not, are direct pushes to a branch other than master permitted?
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 I think we need to clarify this more. Master (code samples) and gh-pages should both not be pushed to directly. Archived should also probably be not pushed to.
My opinion is creating dev branches in the repo works for maintainers, makes it easier to collaborate among ourselves compared to forking -> PR. Here direct pushes should be allowed. Contributors would do the fork->PR workflow.
### How are maintainers removed? | ||
|
||
When a maintainer is unable to perform the [required duties](#what-are-a-maintainers-responsibilities) they can be removed by the [governance procedure](GOVERNANCE.md). | ||
Issues related to a maintainer's performance should be discussed with them among the other maintainers so that they are not surprised by a pull request removing them. |
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.
Does this sentence belong in the governance procedure under Removing maintainers, or here?
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.
Governance makes more sense, but a quick link here doesn't hurt either.
I think we might want to reverse the content of these files based on looking at a couple other repos. It looks like https://github.com/goharbor/community |
|
||
### I'm a maintainer, should I make pull requests too? | ||
|
||
Yes. Nobody should ever push to master directly. All changes should be |
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.
We should add branch rules to prevent pushing to master and the gh-pages branch. Also I suggest we change master branch to something more descriptive.
Some examples for a section that outlines our requirements/process for approving pull requests: https://www.prestashop-project.org/maintainers-guide/reviewing-pull-requests/ |
|Maintainer|Role| | ||
|---|---| | ||
|*Dan Harris|CSC Operations| | ||
|*Kevin Sayers|CSC Applications| |
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.
We probably need to update this table
### How are maintainers removed? | ||
|
||
When a maintainer is unable to perform the [required duties](#what-are-a-maintainers-responsibilities) they can be removed by the [governance procedure](GOVERNANCE.md). | ||
Issues related to a maintainer's performance should be discussed with them among the other maintainers so that they are not surprised by a pull request removing them. |
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.
Governance makes more sense, but a quick link here doesn't hurt either.