-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Sort flags alphabetically #4062
Conversation
Signed-off-by: Wiard van Rij <wiard@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Can we try running make docs
to update our docs? 🤗
Yea, I wanted to review this first and discuss it before making it final :) |
Signed-off-by: Wiard van Rij <wiard@outlook.com>
I don't know why the test is failing. Local:
->
Committed all the docs edit;
Guess I have something odd on my machine... will manually fix this. |
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Now all works, just flaky test, retried |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! LGTM!
cc @idoqo
It's because I cannot access /tmp locally - non-root (: It makes up some funky /var directory. I manually fixed this (: |
Right, looks great! Will update the docs in #3993 and see how that goes. Thanks @wiardvanrij |
* sort flags alphabetical Signed-off-by: Wiard van Rij <wiard@outlook.com> * Update changelog, docs and rename sort function Signed-off-by: Wiard van Rij <wiard@outlook.com> * fixes docs, something odd on my machine Signed-off-by: Wiard van Rij <wiard@outlook.com> Signed-off-by: someshkoli <kolisomesh27@gmail.com>
Signed-off-by: Wiard van Rij wiard@outlook.com
Relates to #4034
It sorts the flags alphabetical
Changes
The latest release of the kingpin package is from 17 Dec 2017. It was missing any method to do some funky stuff to get this done. That is, unless we want to manually order all flags in the code. So;
UsageFuncs
becomes availableI think this might fall under "it's not stupid if it works" but this change felt nasty lol. Open for review anyway, LMK.
Verification
Locally tested.
Will update further if this is the way to go...