fix: mr.cancel_merge_when_pipeline_succeeds()#2350
Merged
Conversation
JohnVillalovos
force-pushed
the
jlvillal/cancel_merge
branch
from
4184511 to
a07a94e
Compare
Codecov Report
@@ Coverage Diff @@
## main #2350 +/- ##
==========================================
+ Coverage 95.78% 95.88% +0.09%
==========================================
Files 79 79
Lines 5249 5248 -1
==========================================
+ Hits 5028 5032 +4
+ Misses 221 216 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
nejch
requested changes
Oct 30, 2022
Member
nejch
left a comment
nejch
left a comment
There was a problem hiding this comment.
Thanks @JohnVillalovos nice catch with the response gotcha! I think we just need to update the docstring as well.
Interesting, GitlabMROnBuildSuccessError also sounds a bit weird to me now here, could be there are some historical reasons for discrepancies here.
JohnVillalovos
force-pushed
the
jlvillal/cancel_merge
branch
from
a07a94e to
19ee535
Compare
lmilbaum
reviewed
Oct 31, 2022
lmilbaum
reviewed
Oct 31, 2022
nejch
approved these changes
Oct 31, 2022
Member
nejch
left a comment
nejch
left a comment
There was a problem hiding this comment.
Thanks @JohnVillalovos for the quick fix and @lmilbaum for the review, let's tackle the follow-up separately :)
* Call was incorrectly using a `PUT` method when should have used a
`POST` method.
* Changed return type to a `dict` as GitLab only returns
{'status': 'success'} on success. Since the function didn't work
previously, this should not impact anyone.
* Updated the test fixture `merge_request` to add ability to create
a pipeline.
* Added functional test for `mr.cancel_merge_when_pipeline_succeeds()`
Fixes: #2349
nejch
force-pushed
the
jlvillal/cancel_merge
branch
from
19ee535 to
cd31cda
Compare
lmilbaum
approved these changes
Oct 31, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PUTmethod when should have used aPOSTmethod.dictas GitLab only returns {'status': 'success'} on success. Since the function didn't work previously, this should not impact anyone.merge_requestto add ability to create a pipeline.mr.cancel_merge_when_pipeline_succeeds()Fixes: #2349