Skip to content

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

Conversation

robinson96
Copy link
Contributor

Addresses #1199.

@robinson96 robinson96 force-pushed the feature/project_merge_request_approval_rules branch from 84b4735 to c6fbf39 Compare September 30, 2020 15:46
@robinson96 robinson96 changed the title WIP: Feature/project merge request approval rules Feature/project merge request approval rules Sep 30, 2020
@max-wittig
Copy link
Member

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!

@robinson96
Copy link
Contributor Author

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

@robinson96
Copy link
Contributor Author

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

@max-wittig
Copy link
Member

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

@robinson96
Copy link
Contributor Author

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!

@robinson96
Copy link
Contributor Author

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?

Copy link
Member

@max-wittig max-wittig left a 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

approver_group_ids=[653, 654])
approver_group_ids=[653, 654])

Create a new MR-level approval rule or Change existing MR-level approval rule::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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::

@robinson96 robinson96 requested a review from max-wittig October 20, 2020 22:49
@robinson96
Copy link
Contributor Author

@max-wittig - any other concerns?

@robinson96
Copy link
Contributor Author

@max-wittig - any other concerns with this branch?

@max-wittig max-wittig merged commit 6035ca8 into python-gitlab:master Oct 29, 2020
@max-wittig
Copy link
Member

thanks for the PR and sorry for the time it took to merge

@robinson96
Copy link
Contributor Author

@max-wittig - no worries and thanks for walking me through it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants