Skip to content

small fix based on pullapprove document #38

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

Merged
merged 2 commits into from
Mar 11, 2018

Conversation

Mashimiao
Copy link

Other projects may refer to template's pullapprove config file.
I think we'd better based on pullapprove's document.
This will reduce doubts of pullapprove grammar.

Signed-off-by: Ma Shimiao [email protected]

Other projects may refer to template's pullapprove config file.
I think we'd better based on pullapprove's document.
This will reduce doubts of pullapprove grammar.

Signed-off-by: Ma Shimiao <[email protected]>
Copy link
Contributor

@wking wking left a comment

Choose a reason for hiding this comment

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

The always_pending move is basically reverting a5e7e37 from #29. As I said in that commit message, I think we want always_pending under group_defaults because it's a group setting.

I don't really mind about quoting or not. We haven't quoted since .pullapprove.yml landed in 5b3d5d5, but I don't have a problem with adding quoting now.

.pullapprove.yml Outdated
@@ -4,19 +4,20 @@ requirements:
signed_off_by:
required: true

always_pending:
title_regex: '^WIP'
explanation: 'Work in progress...'
Copy link
Contributor

Choose a reason for hiding this comment

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

This line needs a two-space indent (not a four-space indent) for this to be valid YAML.

Copy link
Author

Choose a reason for hiding this comment

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

Oh!!! Thanks, updated.
But it's still very strange. selinux-maintainers really exists on opencontaienrs repo, but pullapprove says not.

Copy link
Contributor

Choose a reason for hiding this comment

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

selinux-maintainers really exists on opencontaienrs repo, but pullapprove says not.

PullApprove says:

selinux-maintainers team not found on this repo

And looking at our selinux repo, it looks like the group is go-selinux-maintainers (e.g. see here and here). I've filed #39 to catch up with the current group name, but the org admins may want to rename the group to match the current repo name.

Copy link
Author

Choose a reason for hiding this comment

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

That's why I said very strange. At https://github.com/orgs/opencontainers/teams/, we can say selinux-maintainers exists. go-selinux-maintainers does not exist.

@caniszczyk caniszczyk merged commit 7fba349 into opencontainers:master Mar 11, 2018
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.

3 participants