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

Feat/merge trains additional #2547#3149

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

Open

isaac-philip wants to merge7 commits intopython-gitlab:main

base:main

Choose a base branch

fromisaac-philip:feat/merge_trains_additional

Open

Feat/merge trains additional #2547#3149

isaac-philip wants to merge7 commits intopython-gitlab:mainfromisaac-philip:feat/merge_trains_additional

Conversation

Copy link

isaac-philip commentedMar 9, 2025

Merge Trains Api additional support added :

Status of Merge Request on Merge Train
Add Merge Request to Merge Train

closes#2547

(can work to update the MR with docs till its reviewed/ required, thanks)

Copy link

codecovbot commentedMar 9, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.32%. Comparing base(378a836) to head(3ac6561).

Additional details and impacted files

@@           Coverage Diff           @@##             main    #3149   +/-   ##=======================================  Coverage   97.32%   97.32%           =======================================  Files          98       98             Lines        6057     6066    +9     =======================================+ Hits         5895     5904    +9  Misses        162      162

Flag	Coverage Δ
api_func_v4	`83.71% <100.00%> (-0.03%)`	⬇️
cli_func_v4	`84.71% <100.00%> (+0.02%)`	⬆️
unit	`90.22% <100.00%> (+0.03%)`	⬆️

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

Files with missing lines	Coverage Δ
gitlab/v4/objects/merge_trains.py	`100.00% <100.00%> (ø)`
gitlab/v4/objects/projects.py	`98.92% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Author

isaac-philip commentedMar 14, 2025

@nejch kindly take a look at this PR.

also if anybody else I should loop in here do share, thanks.

Copy link

Author

isaac-philip commentedMar 29, 2025

@JohnVillalovos please let us know for any changes needed before merge, thanks.

JohnVillalovos requested a review fromCopilot

April 7, 2025 01:50

This comment was marked as outdated.

JohnVillalovos force-pushed thefeat/merge_trains_additional branch from51a93c1 to7630232Compare

April 17, 2025 16:27

isaac-philip added7 commits

June 7, 2025 08:04

feat(functional): merge train -> merge request :> status fetch

761ff67

feat(functional): merge train -> merge request :> status fetch

e10ba0c

feat(functional):python-gitlab#2547merge-train api for status and a…

cb4e015

…dd mrSigned-off-by: Isaac Philip <4974658+isaac-philip@users.noreply.github.com>

feat(functional):python-gitlab#2547merge-train api for status and a…

775cd78

…dd mr - test data modificationsSigned-off-by: Isaac Philip <4974658+isaac-philip@users.noreply.github.com>

feat(functional):python-gitlab#2547merge-train api for status and a…

d444f04

…dd mr - minor changesSigned-off-by: Isaac Philip <4974658+isaac-philip@users.noreply.github.com>

feat(functional):python-gitlab#2547merge-train api for status and a…

60b0ec0

…dd mr - lint formatted by blackSigned-off-by: Isaac Philip <4974658+isaac-philip@users.noreply.github.com>

feat(functional):python-gitlab#2547merge-train api for status and a…

3ac6561

…dd mr - docs addedSigned-off-by: Isaac Philip <4974658+isaac-philip@users.noreply.github.com>

JohnVillalovos force-pushed thefeat/merge_trains_additional branch from7630232 to3ac6561Compare

June 7, 2025 15:04

JohnVillalovos requested a review fromCopilot

June 7, 2025 15:05

CopilotAI reviewed

Jun 7, 2025

View reviewed changes

Copy link

CopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull Request Overview

Adds support for two new Merge Train API operations: querying the status of a merge request on a merge train and adding a merge request to a merge train.

DefineProjectMergeTrainMergeRequest and its manager with GET/POST support
Wire the new manager into theProjectMergeTrain object and expose the import inprojects.py
Add unit tests and update documentation for the new endpoints

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
gitlab/v4/objects/merge_trains.py	Added`ProjectMergeTrainMergeRequest` and its manager; wired`merge_requests` on`ProjectMergeTrain`
gitlab/v4/objects/projects.py	Exposed`ProjectMergeTrainManager` import on`Project`
tests/unit/objects/test_merge_trains.py	Added fixtures and tests for GET/POST operations on merge train merge requests
docs/gl_objects/merge_trains.rst	Documented the new classes and examples for merge train merge requests

Comments suppressed due to low confidence (2)

tests/unit/objects/test_merge_trains.py:99

[nitpick] The test name uses plural 'merge_trains' while the related test below is named 'test_merge_train_add_merge_request'. Consider renaming for consistency.

def test_merge_trains_status_merge_request(

gitlab/v4/objects/projects.py:67

This import is not referenced directly in this file and will trigger a lint unused-import error. Reintroduce '# noqa: F401' to suppress the false-positive lint warning.

from .merge_trains import ProjectMergeTrainManager

tests/unit/objects/test_merge_trains.py

		@@ -46,6 +46,10 @@
		"duration": 70,
		}

		merge_train_update = mr_content.copy()

Copy link

Preview

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Using dict.copy() here performs a shallow copy, so nested structures (like the 'pipeline' dict) remain shared with mr_content and may be mutated unexpectedly. Consider using copy.deepcopy() to isolate test data changes.

Copilot uses AI. Check for mistakes.

Copy link

Member

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Probably a good idea. Since line 51 would modify the underlying dict.

docs/gl_objects/merge_trains.rst

Comment on lines +12 to +13

		+ :class:`gilab.v4.objects.ProjectMergeTrainMergeRequest`
		+ :class:`gilab.v4.objects.ProjectMergeTrainMergeRequestManager`

Copy link

Preview

CopilotAIJun 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Typo in the module path: 'gilab' should be spelled 'gitlab'.

Suggested change

	+:class:`gilab.v4.objects.ProjectMergeTrainMergeRequest`
	+:class:`gilab.v4.objects.ProjectMergeTrainMergeRequestManager`
	+:class:`gitlab.v4.objects.ProjectMergeTrainMergeRequest`
	+:class:`gitlab.v4.objects.ProjectMergeTrainMergeRequestManager`

Copilot uses AI. Check for mistakes.

JohnVillalovos requested changes

Jun 7, 2025

View reviewed changes

Copy link

Member

JohnVillalovos left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks@isaac-philip

Just a few minor changes requested.

Also it could be squashed down to one commit most likely. That can be done when it is merged or you can do it when you update. Either works.

gitlab/v4/objects/merge_trains.py

		_update_method: UpdateMethod = UpdateMethod.POST

		_update_attrs = RequiredOptional(
		optional=("sha", "squash", "when_pipeline_succeeds")

Copy link

Member

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Shouldauto_merge be added?

https://docs.gitlab.com/api/merge_trains/#add-a-merge-request-to-a-merge-train

tests/unit/objects/test_merge_trains.py

		@@ -46,6 +46,10 @@
		"duration": 70,
		}

		merge_train_update = mr_content.copy()

Copy link

Member

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Probably a good idea. Since line 51 would modify the underlying dict.

Labels

None yet

2 participants

Movatterモバイル変換

Feat/merge trains additional #2547#3149

Are you sure you want to change the base?

Feat/merge trains additional #2547#3149

Uh oh!

Conversation

isaac-philip commentedMar 9, 2025

Merge Trains Api additional support added :

Uh oh!

codecovbot commentedMar 9, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

isaac-philip commentedMar 14, 2025

Uh oh!

isaac-philip commentedMar 29, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

CopilotAIJun 7, 2025

Choose a reason for hiding this comment

Uh oh!

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

Uh oh!

CopilotAIJun 7, 2025

Choose a reason for hiding this comment

Uh oh!

JohnVillalovos left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

Uh oh!

JohnVillalovosJun 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecovbot commentedMar 9, 2025•
edited
Loading

JohnVillalovos left a comment•
edited
Loading