Skip to content
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

Disable GitHub review requirement and rely on prow? #22

Closed
jlewi opened this issue Mar 1, 2018 · 5 comments
Closed

Disable GitHub review requirement and rely on prow? #22

jlewi opened this issue Mar 1, 2018 · 5 comments

Comments

@jlewi
Copy link
Contributor

jlewi commented Mar 1, 2018

We are moving towards using OWNERS and Prow to ensure CLs are LGTM'd and approved.

Should we disable for our repositories the GitHub check that PRs are approved? It seems like that just forces users to /lgtm/approve requests twice.

@jlewi
Copy link
Contributor Author

jlewi commented Mar 1, 2018

/cc @ddysher @DjangoPeng @gaocegege

@gaocegege
Copy link
Member

SGTM

@ddysher
Copy link
Member

ddysher commented Mar 1, 2018

@jlewi with prow, we don't need github review requirement, but we want to make sure prow works well for us at first.

@jlewi jlewi changed the title Disable GitHub review requirement now that we rely on prow Disable GitHub review requirement and rely on prow? Mar 2, 2018
@jlewi
Copy link
Contributor Author

jlewi commented Mar 2, 2018

@ddysher Agreed. I think the GitHub review status checks block auto-merge. So if we keep them on people need to approve twice (once with GitHub, once with prow) and possible a third time if Reviewable is enabled as a status check.

So my inclination is to start disabling the GitHub status checks so we can really experiment with the Prow workflow. We can always reenable them if need be.

But now that you convinced me to try auto-merge, I think its fantastic. I think it will significantly reduce the toil around managing PRs.

@gaocegege
Copy link
Member

I think it is time to disable GitHub review feature now, since the bot works well in tf-operator

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

No branches or pull requests

3 participants