- Notifications
You must be signed in to change notification settings - Fork673
fix: adds missing status check methods for merge requests#3128
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
base:main
Are you sure you want to change the base?
fix: adds missing status check methods for merge requests#3128
Uh oh!
There was an error while loading.Please reload this page.
Conversation
af3f374 to6bd04caComparecodecovbot commentedFeb 8, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #3128 +/- ##======================================= Coverage 97.32% 97.32% ======================================= Files 98 98 Lines 6057 6067 +10 =======================================+ Hits 5895 5905 +10 Misses 162 162
Flags with carried forward coverage won't be shown.Click here to find out more.
🚀 New features to boost your workflow:
|
6bd04ca tod0abf43Compare@nejch It doesn't seem like we have a Gitlab API to get a single external status check for a project, only list method is supported. is there a cleaner way this can be approached/implemented? I was actually thinking about raising with Gitlab. |
d0abf43 to0a66ae5Compare0a66ae5 to6eee494Compare5c5a336 tocd81aeaComparecd81aea to9430f4dCompare9430f4d tob8a42d5CompareThere 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.
Pull Request Overview
This PR implements the missing status check methods for merge requests as promised in issue#3103.
- Adds fixtures and tests for listing and updating merge request external status checks
- Introduces new classes for handling merge request status check responses in the API client
- Updates documentation to reflect the new API and naming conventions
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/objects/test_status_checks.py | Added fixtures and tests for external status checks functionality |
| tests/functional/api/test_merge_requests.py | Introduced functional test for setting merge request external status |
| gitlab/v4/objects/status_checks.py | Added new manager and response classes for status check updates |
| gitlab/v4/objects/merge_requests.py | Updated merge request object to include new status check managers |
| docs/gl_objects/status_checks.rst | Revised documentation to reflect new external status check methods |
Comments suppressed due to low confidence (1)
gitlab/v4/objects/status_checks.py:66
- [nitpick] The docstring for the update method in ProjectMergeRequestStatusCheckResponseManager should describe updating a merge request status check response rather than referring to a Label.
"""Update a Label on the server."""| "sha": merge_request.sha, | ||
| } | ||
| ) | ||
| response["status"] == "passed" |
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.
The status update check is currently a no-op; use an assertion (e.g., assert response["status"] == "passed") to properly validate the response.
| response["status"]=="passed" | |
| assertresponse["status"]=="passed" |
Copilot uses AI. Check for mistakes.
| Delete an external status check:: | ||
| status_check.delete(status_check_id) | ||
| external_status_check.delete(externa_status_check_id) |
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.
There is a typo in the variable name 'externa_status_check_id'; it should be 'external_status_check_id'.
| external_status_check.delete(externa_status_check_id) | |
| external_status_check.delete(external_status_check_id) |
Copilot uses AI. Check for mistakes.
Uh oh!
There was an error while loading.Please reload this page.
Implements promised changes in#3103