-
Notifications
You must be signed in to change notification settings - Fork 309
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
Add project governance! #679
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for aya-rs-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@aya-rs/aya-maintainers This PR is aimed to solve some open issues around process. I borrowed a lot of this from bpfd 😏 which in turn was borrowed from some CNCF project or template or something.
The other important part is in
Since this is techincally a governance change, this PR will not merge until we have a 2/3 majority (that 6 of the 8 listed maintainers). Feel free to discuss here or in Discord. |
5961b66
to
c7ed48f
Compare
Explains how Maintainers are selected and their responsibilities. Explains the Pull Request review workflow. Adds config for Mergify to enforce this workflow. Signed-off-by: Dave Tucker <[email protected]>
|
||
Thanks for your help improving the project! | ||
* [New Contributor Guide](#contributing-guide) |
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'm not a native English speaker, but is using capital letters everywhere necessary? I'd rather go with "Contributing guide", "Ways to contribute", "Find an issue" etc.
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.
It's Title Case
We have good first issues for new contributors and help wanted issues suitable | ||
for any contributor. [good first issue](https://github.com/aya-rs/aya/labels/good%20first%20issue) has extra information to | ||
help you make your first contribution. [help wanted](https://github.com/aya-rs/aya/labels/help%20wanted) are issues | ||
suitable for someone who isn't a core maintainer and is good to move onto after | ||
your first pull request. |
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.
And here some sentences start without capital letters. 😛 Anyway, I would still rephrase it entirely.
We have good first issues for new contributors and help wanted issues suitable | |
for any contributor. [good first issue](https://github.com/aya-rs/aya/labels/good%20first%20issue) has extra information to | |
help you make your first contribution. [help wanted](https://github.com/aya-rs/aya/labels/help%20wanted) are issues | |
suitable for someone who isn't a core maintainer and is good to move onto after | |
your first pull request. | |
Issues labelled as ["good first issue"](https://github.com/aya-rs/aya/labels/good%20first%20issue) | |
are suitable for new contributors. They provide extra information to help you make your first | |
contribution. | |
Issues labelled as ["help wanted"](https://github.com/aya-rs/aya/labels/help%20wanted) are | |
usually more difficult. We recommend them for people who aren't core maintainers, but either | |
made some contributions already or feel comfortable with starting from more demanding tasks. |
|
||
The best way to reach us with a question when contributing is to ask on: | ||
|
||
* The original github issue |
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.
GitHub
The best way to reach us with a question when contributing is to ask on: | ||
|
||
* The original github issue | ||
* Our Discord |
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 include an invite link here (and everywhere we mention Discord).
|
||
This is my commit message | ||
|
||
Signed-off-by: Your Name <[email protected]> |
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 really want to require signoffs? @alessandrod
I don't mind it (I did it already in some of my PRs without even thinking about it), but we have a lot of not signed off commits. I'm also not a lawyer, but I don't think is really necessary for our licensing stuff?
- name: automatic merge for Dependabot pull request that pass CI | ||
conditions: | ||
- author=dependabot[bot] | ||
actions: | ||
comment: | ||
message: "@dependabot merge" | ||
|
||
- name: automatic merge conditions for main | ||
conditions: | ||
- "#approved-reviews-by>=2" |
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 really need two reviewers?
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.
Normally I'd say no....but I feel like with there being 10 approvers it won't have nearly as much effect on velocity and will ultimately help stability.
- base=main | ||
- label!=hold | ||
- label!=work-in-progress | ||
- check-success=DCO |
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.
this makes it require the "signed off by" line? what does that do for us?
i think requiring signed commits is a much stronger stance (https://docs.github.com/en/[email protected]/authentication/managing-commit-signature-verification/about-commit-signature-verification) but also considerably more onerous.
I think we should just drop this requirement.
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 feel signing your commits (just the sign-off
line not commit verification) is just the Open source community standard tbh, I'm not against having it.
- name: automatic merge for Dependabot pull request that pass CI | ||
conditions: | ||
- author=dependabot[bot] | ||
actions: | ||
comment: | ||
message: "@dependabot merge" | ||
|
||
- name: automatic merge conditions for main |
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 really want automatic merges? I'd really rather let a human being decide on the merge decision.
Perhaps we can allow folks to merge their own changes after they've been approved (pushing a new revision should require re-approval).
|
||
## Supported Versions | ||
|
||
No released versions of aya or it's subprojects will receive regular security updates until a mainline release has been performed. |
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/it's/its/
## Supported Versions | ||
|
||
No released versions of aya or it's subprojects will receive regular security updates until a mainline release has been performed. | ||
A reported and fixed vulnerability will be included in the next minor release, which depending on the severity of the vulnerability may be immediate. |
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.
the line length here and in the other docs is inconsistent. can we pick one and use it everywhere?
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.
Agreed. And I would suggest to use https://github.com/marketplace/actions/markdown-linting-action
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.
++++
@@ -0,0 +1,16 @@ | |||
# 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.
do we need this file? https://github.com/orgs/aya-rs/people is the source of truth (though we should make everyone's membership public)
To become a Maintainer you need to demonstrate the following: | ||
|
||
- commitment to the project: | ||
- participate in discussions, contributions, code and documentation reviews, for 6 months or more, |
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.
these requirements seem overly rigid to me. we should keep this list as a reference, but i don't think it is helpful to frame it as strictly required.
Maintainers may resign at any time if they feel that they will not be able to | ||
continue fulfilling their project duties. | ||
|
||
Maintainers may also be removed after being inactive, failing to fulfill their |
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 happen without a vote? who is the executor?
The Maintainers will appoint a Security Response Team to handle security reports. | ||
This committee may simply consist of the Maintainer Council themselves. If this | ||
responsibility is delegated, the Maintainers will appoint a team of at least two | ||
contributors to handle it. The Maintainers will review who is assigned to this |
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.
nit: double space after this period should be single
edit: there are a few others, please check them all
@@ -86,3 +155,22 @@ nicely even when it is indented. | |||
Fixes: #1337 | |||
Refs: #453, #154 | |||
``` | |||
|
|||
## Pull Request Checklist |
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.
can we move some or all of this into the github pull request template?
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 for doing this, just a couple nits/comments!
The one thing I don't see mentioned is the fact of joining the org WITHOUT becoming a maintainer. For contributors who just want to be able to manually assign issues and stuff it's nice to be made an org member I think. Something along the lines of what K8s calls a "member"
- name: automatic merge for Dependabot pull request that pass CI | ||
conditions: | ||
- author=dependabot[bot] | ||
actions: | ||
comment: | ||
message: "@dependabot merge" | ||
|
||
- name: automatic merge conditions for main | ||
conditions: | ||
- "#approved-reviews-by>=2" |
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.
Normally I'd say no....but I feel like with there being 10 approvers it won't have nearly as much effect on velocity and will ultimately help stability.
- base=main | ||
- label!=hold | ||
- label!=work-in-progress | ||
- check-success=DCO |
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 feel signing your commits (just the sign-off
line not commit verification) is just the Open source community standard tbh, I'm not against having it.
|
||
## Documentation | ||
If anything doesn't make sense, or doesn't work when you run it, please open a | ||
bug report and let us know! |
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 probably add some issue templates along with this
i.e bug-report, enhancement-request, etc
before you submit your code: | ||
|
||
* That Rust code has been formatted with `cargo +nightly fmt` and that all clippy lints have been fixed - you can find failing lints with `cargo +nightly clippy` | ||
* That Go code has been formatted and linted |
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.
nit: not applicable here :)
* Answering questions on Discord | ||
* Web design | ||
* Communications / Social Media / Blog Posts | ||
* Release management |
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.
Community-management?
Our process is currently as follows: | ||
|
||
1. When you open a PR a maintainer will automatically be assigned for review | ||
1. Make sure that your PR is passing CI - if you need help with failing checks please feel free to ask! |
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.
NIT:
1. Make sure that your PR is passing CI - if you need help with failing checks please feel free to ask! | |
2. Make sure that your PR is passing CI - if you need help with failing checks please feel free to ask! |
And so-on
| [Mary](https://github.com/marysaka) | ? | Compatibility with older kernels | | ||
| [](https://github.com/ajwerner) | ? | ? | | ||
| [Tamir Duberstein](https://github.com/tamird) | ? | ? | | ||
| [Andrew Stoycos](https://github.com/astoycos) | Red Hat | ? | |
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.
Lol Idk maybe
| [Andrew Stoycos](https://github.com/astoycos) | Red Hat | ? | | |
| [Andrew Stoycos](https://github.com/astoycos) | Red Hat | Map API | |
🤷
## Supported Versions | ||
|
||
No released versions of aya or it's subprojects will receive regular security updates until a mainline release has been performed. | ||
A reported and fixed vulnerability will be included in the next minor release, which depending on the severity of the vulnerability may be immediate. |
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.
++++
status. Emeritus Maintainers will still be consulted on some project matters | ||
and can be rapidly returned to Maintainer status if their availability changes. | ||
|
||
## Meetings |
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 know no one is a fan but I feel like even a monthly or bi-monthly community meeting would be nice, I know it's hard to get together but as Aya grows more and more folks will get involved.
@dave-tucker, this pull request is now in conflict and requires a rebase. |
@dave-tucker, this pull request is now in conflict and requires a rebase. |
@dave-tucker, this pull request is now in conflict and requires a rebase. |
Adds documentation to explain:
Code changes:
build-workflow-complete
).TODO before merge:
TODO after merge:
This change is