NotificationsYou must be signed in to change notification settings
Fork675
Star2.4k

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nejch merged 1 commit intomainfromjlvillal/approval_rule_id

Jun 20, 2022

Merged

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

nejch merged 1 commit intomainfromjlvillal/approval_rule_id

Jun 20, 2022

+2 −2

Conversation

Copy link

Member

JohnVillalovos commentedJun 11, 2022•
edited
Loading

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 therepr function now works. Before it
did not.

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

Closes:#2065

JohnVillalovos marked this pull request as draft

June 11, 2022 02:41

JohnVillalovos force-pushed thejlvillal/approval_rule_id branch 4 times, most recently from35f1f61 to5f9d1dcCompare

June 11, 2022 03:10

JohnVillalovos marked this pull request as ready for review

June 11, 2022 03:11

JohnVillalovos requested a review fromnejch

June 11, 2022 03:12

nejch reviewed

Jun 11, 2022

View reviewed changes

Copy link

Member

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.

tests/unit/objects/test_project_merge_request_approvals.py OutdatedShow resolvedHide resolved

gitlab/v4/objects/merge_request_approvals.pyShow resolvedHide resolved

JohnVillalovos force-pushed thejlvillal/approval_rule_id branch from5f9d1dc toed3c342Compare

June 11, 2022 14:33

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

15a242c

Using the CLI the command:   gitlab project-merge-request-approval-rule list --mr-iid 1 --project-id foo/barWould 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 itdid not.This is an EE feature so we can't functional test it.Closes:#2065

JohnVillalovos force-pushed thejlvillal/approval_rule_id branch fromed3c342 to15a242cCompare

June 18, 2022 16:05

JohnVillalovos requested a review fromnejch

June 18, 2022 16:06

Copy link

codecov-commenter commentedJun 18, 2022•
edited by codecovbot
Loading

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