-
Notifications
You must be signed in to change notification settings - Fork 671
feat: Added approve method for Mergerequests #685
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
Conversation
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, the code looks good!
Could you have a look at the pep8 failures (missing blank lines).
GitLab EE also provides an unapprove
method that could be added here. But it's OK if you don't do it :)
I have added the unapprove method and I have put the parameters on one line, added default value for sha. I could not find the pep8 errors about double missing blank lines? (I ran tox locally against my branch). |
@gpocentek Travis is not reporting back and I don't see the build in the UI. Do you have any way to restart it? |
I am not very familiar with travis, I have closed and reopened the pull request. That should trigger another build? |
A force push would trigger another build. But I think @gpocentek should be able to restart it manually. |
I can't see the build on travis. @Joustie Do you mind doing a rebase and a push --force? Thanks! |
2d1a3ac
to
757a2d8
Compare
@gpocentek I have made sure the travis builds succeed for my fork, and rebased and pushed --force but still the checks are not updated? Maybe try a brandnew PR? |
Offical GitLab API supports this for GitLab EE
@Joustie I'm sorry that travis is misbehaving. I think we should merge it. Tests pass locally for me. @Joustie Could you do a rebase and then @gpocentek could merge it. (not sure, if I could as CI is failing) |
@Joustie the changes look good and we'll merge without travis, but there are merge conflicts that need to be resolved first. Could you have a look at that? Thanks! |
Offical GitLab API supports approval for GitLab EE