FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Added approve method for Mergerequests by Joustie · Pull Request #685 · python-gitlab/python-gitlab · GitHub

Repository navigation

feat: Added approve method for Mergerequests - #685

Merged
max-wittig merged 2 commits into
python-gitlab:masterfrom
Joustie:master
Jan 21, 2019
Merged

max-wittig merged 2 commits into
python-gitlab:masterfrom
Joustie:master

Conversation

Joustie commented Jan 17, 2019

Copy link
Copy Markdown
Contributor

Offical GitLab API supports approval for GitLab EE

gpocentek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread gitlab/v4/objects.py Outdated
gpocentek added this to the v1.8.0 milestone Jan 19, 2019

Joustie commented Jan 19, 2019

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
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 closed this Jan 19, 2019
Joustie reopened this Jan 19, 2019

Joustie commented Jan 19, 2019

Copy link
Copy Markdown
Contributor Author

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

Joustie closed this Jan 19, 2019
Joustie reopened this Jan 19, 2019

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor

I can't see the build on travis.

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

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

Joustie commented Jan 19, 2019

Copy link
Copy Markdown
Contributor Author

@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 commented Jan 20, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
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 merged commit 641b80a into python-gitlab:master Jan 21, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL