Add common short version for help of CLI

Nitrokey / pynitrokey

Python client for Nitrokey devices

Apache License 2.0

94 stars 28 forks source link

Add common short version for help of CLI #509

Closed anotherbridge closed 2 months ago

anotherbridge commented 3 months ago

Added a short version to call the help, i.e. now the help of a functionality is available by calling --help and -h.

Changes

Added help option names in the context settings of the click group added to the nitropy command.

Checklist

[x] run make check
[x] run make fix
[x] tested with Python3.9
[x] signed commits
[ ] updated documentation (e.g. parameter description, inline doc, docs.nitrokey)
- Nothing to update in my point of view, please correct me if I'm wrong.
[ ] added labels

Test Environment and Execution

OS: macOS Sonoma 14.3.1
device's model: Nitrokey 3C NFC
device's firmware version: v1.6.0-test.20231218

daringer commented 2 months ago

not sure if it's a good idea to apply make fix here, as obviously change is not really atomic anymore - what do you think @sosthene-nitrokey or @robin-nitrokey ?

robin-nitrokey commented 2 months ago

I think it’s fine. The commits are atomic, and atomic PRs is something that is neither achievable nor even desirable.

Though I wonder why these changes occur in the first place – some linting dependency that we didn’t pin hard enough? New Python version?

sosthene-nitrokey commented 2 months ago

I think we should manually cherry-pick a0ce77970ad9ecf0f3e0e02a7133724eef977195 instead of merging.

robin-nitrokey commented 2 months ago

I’m really confused now. Why can’t we start a CI run for this PR? And why did make fix perform these changes in the first place? When testing locally, make check fails with these changes, so we should not merge them.

daringer commented 2 months ago

weird, I would also expect that the CI should run this.... https://github.com/Nitrokey/pynitrokey/blob/600527eac2acb25e811a8e1bbedf8d4a09431b28/.github/workflows/ci.yaml#L6

but, ok let's only include the first commit, @anotherbridge can you be so kind and remove the make fix commit, just include https://github.com/Nitrokey/pynitrokey/pull/509/commits/a0ce77970ad9ecf0f3e0e02a7133724eef977195 in this PR please - think something went wrong with your python version maybe ?

robin-nitrokey commented 2 months ago

For first-time contributors, CI runs need to be approved. I did that recently, but I don’t remember if it was for this PR. Anyway, either the CI run should show up, or the option to approve and trigger it.

anotherbridge commented 2 months ago

@daringer @robin-nitrokey @sosthene-nitrokey I removed the commit that includes the make fix and also merged your master branch into my fork.

robin-nitrokey commented 2 months ago

Rebased and merged. Thank you!