Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(cli): allow exclusive arguments as optional#2770

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

Merged

Conversation

Sjord
Copy link
Contributor

The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.

Closes#2769

Changes

Documentation and testing

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

@codecovCodecov
Copy link

codecovbot commentedJan 21, 2024
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base(48726fd) 96.48% compared to head(e435fc9) 96.49%.

Additional details and impacted files
@@           Coverage Diff           @@##             main    #2770   +/-   ##=======================================  Coverage   96.48%   96.49%           =======================================  Files          90       90             Lines        5866     5874    +8     =======================================+ Hits         5660     5668    +8  Misses        206      206
FlagCoverage Δ
api_func_v482.20% <0.00%> (-0.12%)⬇️
cli_func_v483.55% <62.50%> (-0.03%)⬇️
unit88.27% <100.00%> (+0.01%)⬆️

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

FilesCoverage Δ
gitlab/v4/cli.py91.69% <100.00%> (+0.22%)⬆️

@JohnVillalovos
Copy link
Member

Would it make more sense to create a mutually exclusive group and add the exclusive args to that?

https://docs.python.org/3/library/argparse.html#mutual-exclusion

Sjord reacted with thumbs up emoji

@Sjord
Copy link
ContributorAuthor

I've changed the code to create a mutually exclusive group.

I think the data model of RequiredOptional is not ideal. Whether options are exclusive should be perpendicular to whether they are required or optional. Also, it would be possible to have multiple exclusive groups. Currently this is not reflected in the RequiredOptional.exclusive property. It only allows one group, and it is not clear whether this group is required or not.

Ideally, you would have something lilke this:

RequiredOptional(    required=("foo", exclusive("user_id", "user_email"), exclusive("project_slug", "project_id")),    optional=("bar", exclusive("cn", "filter")))

But adding these exclusive options to the CLI argument parser at least makes it functioning again, if not perfect.

@SjordSjordforce-pushed theallow-exclusive-arguments branch from24dc41e toa53397dCompareJanuary 28, 2024 20:37
The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.Closespython-gitlab#2769
Copy link
Member

@JohnVillalovosJohnVillalovos left a comment

Choose a reason for hiding this comment

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

Thank you very much for this.

@JohnVillalovosJohnVillalovos merged commit7ec3189 intopython-gitlab:mainJan 29, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@JohnVillalovosJohnVillalovosJohnVillalovos approved these changes

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

Successfully merging this pull request may close these issues.

Exclusive arguments not allowed in CLI
2 participants
@Sjord@JohnVillalovos

[8]ページ先頭

©2009-2025 Movatter.jp