-
Notifications
You must be signed in to change notification settings - Fork 671
feat: Add reset_approvals api #2279
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
feat: Add reset_approvals api #2279
Conversation
d725e2c
to
8a677b5
Compare
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.
Thanks a lot @zampierilucas, looks great, I mainly just had a few style comments for docs.
I see you're getting a 404 on the functional test, did this work locally?
I've retried just in case, but I'm not sure if you need to first ensure that an approval exists for this to work, or if we maybe have an issue because this is done with an admin token and not a resource access token that this endpoint requires.
Oh I'm sorry @zampierilucas the only reason this is probably failing is that we're on an old GitLab image. Could you bump the docker image version here? Something's wrong with our renovate setup, this was supposed to be auto-updated. After that's done, let's see if it works with an admin token or if we get a 401 as well in tests. Thanks again :) |
I have created a PR to update the Docker image: #2280 |
8a677b5
to
2a14b20
Compare
Thank you both @nejch and @JohnVillalovos for your feedback, I've applied @nejch suggestion to my last push, and rebase this branch on top of |
2a14b20
to
4408e76
Compare
Thanks @zampierilucas and @JohnVillalovos! I merged the other PR and we can start fresh here. So now we are hitting 401 as I suspected above because we run it as the
I think adding the project access token will be quicker and would also test behavior against future gitlab versions so I think that might be nicer :) let us know if you can try and add that. See docs here: https://python-gitlab.readthedocs.io/en/stable/gl_objects/project_access_tokens.html#examples |
Added the newly added reset_approvals merge request api. Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
4408e76
to
fc7b40c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2279 +/- ##
=======================================
Coverage 95.28% 95.29%
=======================================
Files 79 79
Lines 5238 5246 +8
=======================================
+ Hits 4991 4999 +8
Misses 247 247
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Thanks for the quick rework @zampierilucas! :) This should be available with the next release on the 28th.
Added the newly added reset_approvals merge request api.
https://docs.gitlab.com/ee/api/merge_requests.html#reset-approvals-of-a-merge-request
Signed-off-by: Lucas Zampieri lzampier@redhat.com