FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(api): add merge request pipeline manager and deprecate mr.pipelines() method by nejch · Pull Request #1323 · python-gitlab/python-gitlab · GitHub

  1. python-gitlab
  2. python-gitlab

Repository navigation

feat(api): add merge request pipeline manager and deprecate mr.pipelines() method - #1323

Merged
JohnVillalovos merged 1 commit into
masterfrom
feat/mr-pipeline-manager
Jun 14, 2021
ConversationCommits (1)ChecksFiles changed
Merged

feat(api): add merge request pipeline manager and deprecate mr.pipelines() method#1323
JohnVillalovos merged 1 commit into
masterfrom
feat/mr-pipeline-manager

Conversation

nejch commented Feb 23, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Closes #1312.
Closes #1239.

todo: technically this breaks the CLI gitlab project-merge-request pipelines and requires project-merge-request-pipelines list instead. I'll see if I can hack the register_custom_action decorator to accept custom action names, not just the current method name. I had to remove it as there would be a namespace clash between the old method and the new manager.

Well, I've managed to hack register_custom_action a little so it takes an optional custom_action argument to override the current function name as the CLI action, so should be backwards compatible, but then realized the old method probably never worked in CLI due to missing CLI args 🤦

Still, now you can do (until 3.0.0 or so, but probably useless anyway):

gitlab project-merge-request pipelines --help
usage: gitlab project-merge-request pipelines [-h] --project-id PROJECT_ID [--sudo SUDO] --iid IID

optional arguments:
  -h, --help            show this help message and exit
  --project-id PROJECT_ID
  --sudo SUDO
  --iid IID

And the new list, with proper help and args:

gitlab project-merge-request-pipeline list --help
usage: gitlab merge-request-pipeline list [-h] [--sudo SUDO] --project-id PROJECT_ID --mr-iid MR_IID
                                          [--page PAGE] [--per-page PER_PAGE] [--all]

optional arguments:
  -h, --help            show this help message and exit
  --sudo SUDO
  --project-id PROJECT_ID
  --mr-iid MR_IID
  --page PAGE
  --per-page PER_PAGE
  --all

nejch mentioned this pull request Feb 23, 2021
Closed
nejch force-pushed the feat/mr-pipeline-manager branch 3 times, most recently from c8fed5a to f12841f Compare February 26, 2021 19:35
nejch marked this pull request as ready for review February 26, 2021 19:49

nejch commented Feb 26, 2021

Copy link
Copy Markdown
Member Author

Maybe @JohnVillalovos you want to have a look at the typing here as well, I always forget decorators and have to google the wrap functions every time I work with them 🤣

JohnVillalovos commented Feb 26, 2021

Copy link
Copy Markdown
Member

Maybe @JohnVillalovos you want to have a look at the typing here as well, I always forget decorators and have to google the wrap functions every time I work with them 🤣

From my initial look it seems correct. Also mypy is checking that gitlab/cli.py file, so seems good.

Looks Good To Me (LGTM) 👍

Thanks!

max-wittig commented Feb 26, 2021

Copy link
Copy Markdown
Member

We're most likely gonna go to 3.0.0 directly, because of this breaking change: #1278. Maybe have those two in one release would be nice 😄

msune commented Mar 29, 2021

Copy link
Copy Markdown

👍 on this.

I seem to be hitting a limitation/bug of pipelines() #1312 (comment) and in another issue that I will not bother mentioning.

Any chance this could be integrated soon?

nejch added this to the v3.0.0 milestone Apr 27, 2021

max-wittig commented May 31, 2021

Copy link
Copy Markdown
Member

@nejch Sorry for making you wait for long that everything conflicts. I just see like 15 notifications everyday from this project. You people are crazy 😄

Would you mind to rebase?

max-wittig assigned nejch and unassigned max-wittig May 31, 2021
nejch force-pushed the feat/mr-pipeline-manager branch from f12841f to abe4242 Compare June 13, 2021 18:00
JohnVillalovos requested changes Jun 13, 2021
View reviewed changes

JohnVillalovos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Overall looks good to me. Left one thing I think should be fixed.

Comment thread gitlab/cli.py Outdated
nejch force-pushed the feat/mr-pipeline-manager branch from abe4242 to 954357c Compare June 13, 2021 19:19
JohnVillalovos approved these changes Jun 13, 2021
View reviewed changes

JohnVillalovos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks. This looks good.

I'll let you merge it or if you want to wait for @max-wittig to review. Your choice.

JohnVillalovos merged commit e77554c into master Jun 14, 2021
JohnVillalovos deleted the feat/mr-pipeline-manager branch June 14, 2021 17:58
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JohnVillalovos JohnVillalovos approved these changes

Assignees

nejch

Labels

None yet

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

ProjectMergeRequest.pipelines() returns a 'list' instead of documented 'RESTObjectList' Add function to create pipeline for merge request

4 participants

Back | FazBrowse Home | New Git URL