-
Notifications
You must be signed in to change notification settings - Fork 27
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
.pullapprove.yml: List project maintainers separately #29
.pullapprove.yml: List project maintainers separately #29
Conversation
f6dbf92
to
e4ca9f0
Compare
The “.pullapprove.yml proposed by this PR is invalid” error is because each of the teams will need to be assigned to this repository. From the PullApprove page:
|
@caniszczyk can we adopt this? opencontainers/image-spec#609 (comment) is kinda annoying |
@jonboulle @wking sure, we also need to enable the "github_reviews" option to ensure that those are counted: https://docs.pullapprove.com/groups/github_reviews/ |
On Thu, Mar 16, 2017 at 07:53:21AM -0700, Chris Aniszczyk wrote:
… we also need to enable the "github_reviews" option to ensure that
those are counted:
https://docs.pullapprove.com/groups/github_reviews/
I don't think we need to. From [1]:
GitHub review integration is enabled by default but can be disabled
if need be…
Still, I'm happy to add the setting if we want to make that explicit.
I've pushed e4ca9f0 → 96ab978 adding the selinux-maintainers team.
[1]: dropseed/pullapprove-support#98 (comment)
|
.pullapprove.yml
Outdated
approve_by_comment: | ||
enabled: true | ||
approve_regex: ^LGTM | ||
reject_regex: ^Rejected |
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.
While we're doing this, can we make this NACK? 😉
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.
While we're doing this, can we make this NACK?
My preference would be to use LGTM and REJECT (what does NACK even mean? "Not acknowledged" sounds like "I'm refusing to admit this exists" and not like "I don't like this"). But I don't really care what the terms are as long as they're special enough to not be accidental.
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.
Well, NACK is how LKML does it. I don't really mind but it would be nice if it was all-caps so that it stands out in the text.
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.
fwiw I always took "nack" as "negative acknowledgement" (I acknowledge reviewing this and it should not be shipped at this time)
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.
fwiw I always took "nack" as "negative acknowledgement"…
Good enough for me. I've spun this off into #32 to avoid distracting from this PR.
.pullapprove.yml
Outdated
author_approval: | ||
ignored: true | ||
always_pending: | ||
title_regex: 'WIP' |
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 be ^WIP.
.pullapprove.yml
Outdated
required: 2 | ||
reject_value: -1 |
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 sure we should have this, since it means you'd need 3 LGTMs to get a NACK'd PR merged.
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 sure we should have this, since it means you'd need 3 LGTMs to get a NACK'd PR merged.
That's why I put it in ;). It seems like there should be a distinction between abstains/no-review and "I looked at this and didn't like it".
But I'm happy to pull 0a60167 out into a follow-up PR if it would help land the rest of 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.
I've spun this point off into #31 so we can land the rest of this PR.
96ab978
to
c87ca52
Compare
Docs in http://docs.pullapprove.com/groups/always_pending/ The ^ prefix (anchoring the match to the front of the title) is Aleksa's suggestion [1]. [1]: opencontainers#29 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
c87ca52
to
06f8c77
Compare
Can you squash this into one commit and rebase this? |
I'm fine with REJECT, but Aleksa suggested a switch to NACK [1] to match the Linux Kernel's usage [2,3]. Jon suggests it may mean “negative acknowledgement” [4]. I've updated both GOVERNANCE and the PullApprove config because it seems better to have consistent keywords (so maintainers don't have to remember which context-dependent string keyword should use). [1]: opencontainers#29 (comment) [2]: opencontainers#29 (comment) [3]: http://lkml.iu.edu/hypermail/linux/kernel/0706.0/0303.html [4]: opencontainers#29 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
06f8c77
to
7233104
Compare
7233104
to
8feb0fd
Compare
This repository is managed by the union of all OCI Project maintainers. To avoid having a few active maintainers from one project driving the template in ways that other projects don't support, require at least two approvals from each project. This will slow down acceptance a bit, but many maintainers belong to multiple projects (and their votes will count for each group [1]), so you won't need 12 separate maintainers voting to get a single project-template PR across the line. Also update from config format v1 to v2 [1,2]. [1]: http://docs.pullapprove.com/migrating-to-v2/ [2]: http://docs.pullapprove.com/ Signed-off-by: W. Trevor King <wking@tremily.us>
Docs in http://docs.pullapprove.com/groups/always_pending/ The ^ prefix (anchoring the match to the front of the title) is Aleksa's suggestion [1]. [1]: opencontainers#29 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
Docs in http://docs.pullapprove.com/groups/conditions/ Signed-off-by: W. Trevor King <wking@tremily.us>
The main example has it outside [1], but it's a group setting [1], so we can move it inside for consistency [2]. [1]: http://docs.pullapprove.com/ [2]: http://docs.pullapprove.com/pr-settings/group_defaults/ Signed-off-by: W. Trevor King <wking@tremily.us>
This project was initially approved as go-selinux [1] but has since renamed itself to drop the go- prefix [2]. [1]: https://groups.google.com/a/opencontainers.org/forum/#!topic/tob/nH8QU6UnrqA Subject: VOTE Required: approve new project go-selinux Date: Fri, 17 Feb 2017 01:21:37 +0000 Message-ID: <CAD2oYtPzgJKeiKKxFnds-yH+2OG3WF6d36gR1uD2Y3+QQdsgpw@mail.gmail.com> [2]: opencontainers/selinux#6 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
Catching up with opencontainers/tob@0057edc (Merge pull request opencontainers#29 from opencontainers/selinux-project, 2017-02-23) and the subsequent rename [1]. I'd still rather drop the parenthetical entirely and link to a place that listed OCI Projects, but we don't have a canonical target for that yet (opencontainers/tob#2) and the current closest instance seems to be the GitHub section in [2] (which doesn't have the "OCI Project" words). [1]: opencontainers/selinux#6 (comment) [2]: https://www.opencontainers.org/community Signed-off-by: W. Trevor King <wking@tremily.us>
@wking it's a preference thing but I don't have energy right now to fight the request |
Looks like we're still missing an |
I'm fine with REJECT, but Aleksa suggested a switch to NACK [1] to match the Linux Kernel's usage [2,3]. Jon suggests it may mean “negative acknowledgement” [4]. I've updated both GOVERNANCE and the PullApprove config because it seems better to have consistent keywords (so maintainers don't have to remember which context-dependent string keyword should use). [1]: opencontainers#29 (comment) [2]: opencontainers#29 (comment) [3]: http://lkml.iu.edu/hypermail/linux/kernel/0706.0/0303.html [4]: opencontainers#29 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
This lazily copies image-spec's configuration as updated via opencontainers/project-template#29 and opencontainers/image-spec#616 Signed-off-by: Jonathan Boulle <jonathanboulle@gmail.com>
Pull in changes from [1]. The only changes vs. the upstream version is removing all the groups except for the runtime-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
Pull in changes from [1]. The only changes vs. the upstream version are: * Kept the image-spec-specific approve_regexp. * Removed all the groups except for the image-spec group. [1]: opencontainers/project-template#29 Signed-off-by: W. Trevor King <wking@tremily.us>
This repository is managed by the union of all OCI Project maintainers. To avoid having a few active maintainers from one project driving the template in ways that other projects don't support, require at least two approvals from each project. This will slow down acceptance a bit, but many maintainers belong to multiple projects (and their votes will count for each group), so you won't need 12 separate maintainers voting to get a single project-template PR across the line.
A side benefit of this is that we no longer need to maintain the tdc-maintainers team.
Also update from config format v1 to v2 and pull in some new features (which I can spin off into separate PRs if they turn out to be controversial).