Skip to content

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

Merged
merged 2 commits into from
Jan 21, 2019

Conversation

Joustie
Copy link
Contributor

@Joustie Joustie commented Jan 17, 2019

Offical GitLab API supports approval for GitLab EE

Copy link
Contributor

@gpocentek gpocentek left a 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 :)

@gpocentek gpocentek added this to the v1.8.0 milestone Jan 19, 2019
@Joustie
Copy link
Contributor Author

Joustie commented Jan 19, 2019

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).

@max-wittig
Copy link
Member

@gpocentek Travis is not reporting back and I don't see the build in the UI. Do you have any way to restart it?

@Joustie Joustie closed this Jan 19, 2019
@Joustie Joustie reopened this Jan 19, 2019
@Joustie
Copy link
Contributor Author

Joustie commented Jan 19, 2019

I am not very familiar with travis, I have closed and reopened the pull request. That should trigger another build?

@Joustie Joustie closed this Jan 19, 2019
@Joustie Joustie reopened this Jan 19, 2019
@max-wittig
Copy link
Member

A force push would trigger another build. But I think @gpocentek should be able to restart it manually.

@gpocentek
Copy link
Contributor

I can't see the build on travis.

@Joustie Do you mind doing a rebase and a push --force? Thanks!

@Joustie Joustie force-pushed the master branch 3 times, most recently from 2d1a3ac to 757a2d8 Compare January 19, 2019 20:13
@Joustie Joustie closed this Jan 19, 2019
@Joustie Joustie reopened this Jan 19, 2019
@Joustie
Copy link
Contributor Author

Joustie commented Jan 19, 2019

@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
@max-wittig
Copy link
Member

max-wittig commented Jan 20, 2019

@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)

@gpocentek
Copy link
Contributor

@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!

@max-wittig max-wittig merged commit 641b80a into python-gitlab:master Jan 21, 2019
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