Skip to content

Conversation

@GDWR
Copy link
Contributor

@GDWR GDWR commented Aug 16, 2025

This Pull Request updates the handling of the --version & version subcommand to exit immediately with a 0 (successful) exit code, in scenarios that the version is correctly output. 1 (failure) will still be emitted when the version cannot be retrieved correctly.

I've noticed this while outputting debug information in pipelines, as it causes the task to fail. I can always ignore the unsuccessful exit || true etc, but I think outputting the correct exit code would be ideal.


I did have a look at utilising the "Args" return to carry the exit code back to Program.cs for handling the exiting of the application in unified manner, but it feels like a misuse of "Args" as they're intended to be client driven. If this approach is desired, from the structure of the application I would suggest moving the handling of version into a VersionService to unify how the handling of commands and subsequent exit codes are across the application. As the version logic is already different to the rest in where it is handled, I did not think this refactor would be desired. I am happy to refactor into a service if that feels more appropriate.

@GDWR GDWR requested a review from a team as a code owner August 16, 2025 09:42
@GDWR
Copy link
Contributor Author

GDWR commented Aug 16, 2025

@microsoft-github-policy-service agree

@DaveTryon
Copy link
Contributor

/azp run

Copy link
Contributor

@DaveTryon DaveTryon left a comment

Choose a reason for hiding this comment

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

LGTM. It will probably need to be to the latest commit before it can be merged

@GDWR
Copy link
Contributor Author

GDWR commented Aug 18, 2025

LGTM. It will probably need to be to the latest commit before it can be merged

I was going to rebase the PR, but it seems like its already based on the latest commit in main.
Is that what you wanted me to do?

@DaveTryon
Copy link
Contributor

DaveTryon commented Aug 18, 2025

@GDWR Yep, my mistake. We had a commit on Friday and I mistakenly thought that it had merged after your base commit. Merging now. Thanks for the contribution!

Version 4.1.1 should release later this week. Your change will probably be in release 4.1.2 (no ETA at the moment)

@DaveTryon DaveTryon merged commit 0a278ab into microsoft:main Aug 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants