-
Notifications
You must be signed in to change notification settings - Fork 669
Feature/project merge request approval rules #1200
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
Feature/project merge request approval rules #1200
Conversation
84b4735
to
c6fbf39
Compare
Hi @robinson96 thanks for the MR! Would you mind to add some docs and tests to this? I can't really test EE features, but I will take a look based on your docs! |
I will certainly add docs. I too lack a great way to do EE testing, I develop GRAPE, which has some features for maintaining pull requests. It works in that context. (branch unfortunately is on a repo hosting service that is not public). |
@max-wittig , I'm trying to find test_merge_requests.py so I can modify it to add appropriate tests, but am coming up short. (My python testing skills are still developing). Any hints on where to be looking? (tools/functional/api/ only has a few of the files that Travis appears to be exercising...) |
Our unittests are here: https://github.com/python-gitlab/python-gitlab/tree/master/gitlab/tests/objects And functional tests are here: https://github.com/python-gitlab/python-gitlab/tree/master/tools/functional/api For this, it's only possible to add unit tests, as we run the functional tests against GitLab Core |
Thanks. Tests are added! |
@max-wittig - any other concerns with this PR? |
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.
Ah yes sorry I forgot to press the submit review button
docs/gl_objects/mr_approvals.rst
Outdated
approver_group_ids=[653, 654]) | ||
approver_group_ids=[653, 654]) | ||
|
||
Create a new MR-level approval rule or Change existing MR-level approval rule:: |
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.
Create a new MR-level approval rule or Change existing MR-level approval rule:: | |
Create a new MR-level approval rule or change an existing MR-level approval rule:: |
@max-wittig - any other concerns? |
@max-wittig - any other concerns with this branch? |
thanks for the PR and sorry for the time it took to merge |
@max-wittig - no worries and thanks for walking me through it! |
Addresses #1199.