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.

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

feat: add --bulk option to add task command #11

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

u8slvn
Copy link
Owner

@u8slvn u8slvn commented Jul 31, 2024

#9

@u8slvn u8slvn requested a review from a-delannoy July 31, 2024 08:47
@u8slvn u8slvn self-assigned this Jul 31, 2024
"--details",
"details",
cls=NotRequired,
not_required=["bulk"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be not_required_if ?

Copy link
Owner Author

@u8slvn u8slvn Aug 1, 2024

Choose a reason for hiding this comment

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

It's two different classes, see here: parameters.py

The first one NotRequiredIf is a click.Argument class, it allows marking a command arg as "not required if" the following parameters are present. The second one NotRequired is a click.Option class, it raises an error if one of the given parameters is present with the option.

I guess it's time for me to add some docstrings!

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I understood you well, it means that the title argument is not required when the bulk option is setup. But what if both title and bulk are presents ? Shouldn't it raise an error ?

I mean, the bulk option seems to be incompatible with both details and title since those two last arguments refer to a single task, while bulk introduces several tasks.

If indeed it is incompatible, either one should raise an error when setup along with bulk, what do you think ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is already how it works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok fine.

@u8slvn u8slvn requested a review from a-delannoy August 7, 2024 13:49
@u8slvn u8slvn added the enhancement New feature or request label Aug 8, 2024
@u8slvn u8slvn merged commit 61f3fe9 into main Aug 8, 2024
16 checks passed
@u8slvn u8slvn deleted the feat/bulk-option-for-add-task-cmd branch August 8, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants