Skip to content

feat(api): add merge request approval state #1641

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

Conversation

JacobHenner
Copy link
Contributor

Add support for merge request approval state.

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2021

Codecov Report

Merging #1641 (f41b093) into main (5a1678f) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1641      +/-   ##
==========================================
+ Coverage   91.90%   91.93%   +0.02%     
==========================================
  Files          74       74              
  Lines        4287     4302      +15     
==========================================
+ Hits         3940     3955      +15     
  Misses        347      347              
Flag Coverage Δ
cli_func_v4 81.96% <100.00%> (+0.06%) ⬆️
py_func_v4 80.98% <100.00%> (+0.06%) ⬆️
unit 83.77% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gitlab/v4/objects/merge_request_approvals.py 93.58% <100.00%> (+0.53%) ⬆️
gitlab/v4/objects/merge_requests.py 85.82% <100.00%> (+0.11%) ⬆️
gitlab/v4/objects/runners.py 98.18% <0.00%> (ø)
gitlab/v4/objects/users.py 98.50% <0.00%> (+0.06%) ⬆️

@JacobHenner
Copy link
Contributor Author

The CI failure appears to be unrelated to this change?

@JohnVillalovos
Copy link
Member

Thanks for the PR. It would be great if there can be some tests for this, assuming it is a "Free Tier" feature.

@JacobHenner
Copy link
Contributor Author

Thanks for the PR. It would be great if there can be some tests for this, assuming it is a "Free Tier" feature.

It's a premium feature since 13.9. I was prepared to add unit tests, but I noticed that "merge request approvals" doesn't have any to begin with.

@nejch
Copy link
Member

nejch commented Oct 15, 2021

Thanks for the PR. It would be great if there can be some tests for this, assuming it is a "Free Tier" feature.

It's a premium feature since 13.9. I was prepared to add unit tests, but I noticed that "merge request approvals" doesn't have any to begin with.

Hi @JacobHenner, thanks for this! As @JohnVillalovos said, we won't be able to add functional tests without a license but we can have some unit tests maybe.

Maybe in https://github.com/python-gitlab/python-gitlab/blob/main/tests/unit/objects/test_project_merge_request_approvals.py?
Just a simple get() call and mock response to make sure we keep sending the right request. Thanks again for the super clean PR!

Add support for merge request approval state
@JacobHenner JacobHenner force-pushed the add-merge-request-approval-state branch from 5e07faf to f41b093 Compare October 20, 2021 01:27
@nejch nejch merged commit 422309f into python-gitlab:main Oct 20, 2021
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.

4 participants