Skip to content

fix(cli): project-merge-request-approval-rule #2066

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 1 commit into from
Jun 20, 2022
Merged

Conversation

JohnVillalovos
Copy link
Member

@JohnVillalovos JohnVillalovos commented Jun 11, 2022

Using the CLI the command:
gitlab project-merge-request-approval-rule list --mr-iid 1 --project-id foo/bar

Would raise an exception. This was due to the fact that _id_attr and
_repr_attr were set for keys which are not returned in the response.

Add a unit test which shows the repr function now works. Before it
did not.

This is an EE feature so we can't functional test it.

Closes: #2065

@JohnVillalovos JohnVillalovos marked this pull request as draft June 11, 2022 02:41
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/approval_rule_id branch 4 times, most recently from 35f1f61 to 5f9d1dc Compare June 11, 2022 03:10
@JohnVillalovos JohnVillalovos marked this pull request as ready for review June 11, 2022 03:11
@JohnVillalovos JohnVillalovos requested a review from nejch June 11, 2022 03:12
Copy link
Member

@nejch nejch left a comment

Choose a reason for hiding this comment

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

Thanks @JohnVillalovos! Just a quick question there, and I'll just check a bit later today what that other method is doing with the custom ID.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/approval_rule_id branch from 5f9d1dc to ed3c342 Compare June 11, 2022 14:33
Using the CLI the command:
   gitlab project-merge-request-approval-rule list --mr-iid 1 --project-id foo/bar

Would raise an exception. This was due to the fact that `_id_attr` and
`_repr_attr` were set for keys which are not returned in the response.

Add a unit test which shows the `repr` function now works. Before it
did not.

This is an EE feature so we can't functional test it.

Closes: #2065
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/approval_rule_id branch from ed3c342 to 15a242c Compare June 18, 2022 16:05
@JohnVillalovos JohnVillalovos requested a review from nejch June 18, 2022 16:06
@codecov-commenter
Copy link

codecov-commenter commented Jun 18, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.64%. Comparing base (ae7d3b0) to head (15a242c).
Report is 740 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2066      +/-   ##
==========================================
+ Coverage   94.62%   94.64%   +0.02%     
==========================================
  Files          78       78              
  Lines        4983     5002      +19     
==========================================
+ Hits         4715     4734      +19     
  Misses        268      268              
Flag Coverage Δ
cli_func_v4 82.44% <100.00%> (-0.14%) ⬇️
py_func_v4 81.02% <100.00%> (+0.07%) ⬆️
unit 85.60% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
gitlab/v4/objects/merge_request_approvals.py 90.80% <100.00%> (-0.11%) ⬇️

... and 2 files with indirect coverage changes

@nejch nejch merged commit 41ceaca into main Jun 20, 2022
@nejch nejch deleted the jlvillal/approval_rule_id branch June 20, 2022 15:16
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.

gitlab project-merge-request-approval-rule list throws a KeyError "approval_rule_id" error.
3 participants