Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(api): return the new commit when calling cherry_pick #3097

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(api): return the new commit when calling cherry_pick #3097

wants to merge 1 commit into from

Conversation

mschoettle
Copy link

@mschoettle mschoettle commented Jan 22, 2025 •
edited
Loading

Changes

Returns the response when calling ProjectCommit.cherry_pick.

Documentation and testing

Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated:

Fixes #3096

@mschoettle mschoettle marked this pull request as ready for review January 22, 2025 22:46
@codecov Codecov
Copy link

codecov bot commented Jan 22, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.77%. Comparing base (e8d6953) to head (b4a2852).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3097      +/-   ##
==========================================
+ Coverage   96.72%   96.77%   +0.04%     
==========================================
  Files          96       96              
  Lines        6293     6293              
==========================================
+ Hits         6087     6090       +3     
+ Misses        206      203       -3
Flag Coverage Δ
api_func_v4 82.98% <100.00%> (+0.15%) ⬆️
cli_func_v4 83.01% <50.00%> (ø)
unit 89.25% <100.00%> (+0.04%) ⬆️

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

Files with missing lines Coverage Δ
gitlab/v4/objects/commits.py 98.79% <100.00%> (+3.61%) ⬆️

@mschoettle mschoettle marked this pull request as draft January 23, 2025 00:35
nejch
nejch requested changes Jan 23, 2025
View reviewed changes
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 a lot @mschoettle, makes sense! I think this must be very old code.

Would you mind adding a test for this, maybe just in the functional tests if it's easier than mocking? I know we didn't have one before, but just in case. Let me know if you need help.

@mschoettle mschoettle force-pushed the patch-1 branch 3 times, most recently from b963cc4 to f87d6d4 Compare January 23, 2025 15:18
@mschoettle
Copy link
Author

@nejch Thanks! I noticed yesterday that there were no tests for cherry_pick so I started adding a unit test and functional test based on the existing tests.

I am having an issue with running the functional tests in the codespace so I am relying on the pipeline for those.

@mschoettle mschoettle marked this pull request as ready for review January 23, 2025 16:01
@mschoettle mschoettle requested a review from nejch January 23, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nejch nejch Awaiting requested review from nejch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ProjectCommit.cherry_pick should return the created commit
2 participants
@mschoettle @nejch