woodpecker-ci / woodpecker

Woodpecker is a simple yet powerful CI/CD engine with great extensibility.
https://woodpecker-ci.org
Apache License 2.0
3.79k stars 333 forks source link

Improve cli #3386

Open anbraten opened 3 months ago

anbraten commented 3 months ago

The woodpecker cli could get some polishing:

xoxys commented 1 month ago

@woodpecker-ci/maintainers I would like to work on improve error handling for missing args (missing repo id / name results in an parseInt error atm) How do we want to handle it? Do we really want to continue using args? IMO, it would be better to drop args completely and just use flags. Any reason to keep using args?

anbraten commented 2 weeks ago

I think its quite common practice to use args (see kubectl, docker, ...) therefore I would suggest to stick to it and add an empty-string check with a more descriptive error in front of the parseInt call.

xoxys commented 2 weeks ago

Obviously, but I could also name a lot of tools without args. We could have a way more clean code without arg validation, as flag validation and required flags is a built-in feature of urfave/cli. However, if args is preferred, that works for me as well.

lafriks commented 2 weeks ago

I don't know if urfave/cli still checks that required arguments are not empty if required (you can provide argument but provide it with empty value) so check for empty value could still be needed

xoxys commented 2 weeks ago

There is no required args feature, just a required flags option.