-
Notifications
You must be signed in to change notification settings - Fork 571
feat: Route --version output to stdout #1431
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1431 +/- ##
===========================================
- Coverage 42.19% 28.14% -14.06%
===========================================
Files 54 70 +16
Lines 6456 12540 +6084
===========================================
+ Hits 2724 3529 +805
- Misses 3283 8535 +5252
- Partials 449 476 +27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Instead of reimplementing the Version()
function it looks like we can set the usage and version output to write with os.stdout
using .UsageWriter()
.
So we keep the .Version(
and we set the .UsageWriter(
. I've checked, but we should verify, this should not interfere with error messages caused by incorrect commands from going to stderr because that is handled by fatalUsage()
.
https://github.com/alecthomas/kingpin/blob/64d23c54e3e2a385ce12757939bed9c632d1693c/app.go#L146
Thanks for the review. That's way cleaner. |
I verified that errors are sent to stderr by running this: |
Currently,
saml2aws --version
prints the application version tostderr
. This change modifies the behavior to print the version string tostdout
instead while other logs and errors remain onstderr
.Reasoning:
Printing the version to
stdout
aligns better with common scripting practices and standard CLI tool behavior for version reporting. This makes it easier to capture the version string programmatically without needingstderr
redirection (2>&1
).For example, users with bootstrapping or environment setup scripts that check dependency versions currently need to add special handling for
saml2aws
likeor like
Routing
--version
output tostdout
removes this inconsistency.#1432