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

Allow printing of version number through command-line option #1878

Merged
merged 1 commit into from Aug 25, 2016

Conversation

Projects
None yet
3 participants
@ErikSchierboom
Contributor

ErikSchierboom commented Aug 24, 2016

This PR implements the feature requested in #1875, where you can specify the --version argument to print the version to the console. However, while implementing this, I found I had to remove the RequireSubcommand attribute, as --version is (rightfully) not considered a sub-command and having the RequireSubcommand attribute would cause it to fail. The result of this change is of course that sub-commands are now not necessary, which I think is a bit odd. What do you think about this?

The main problem is that #1875 suggests that the --version flags is used for displaying the version, but perhaps it should just be a regular subcommand?

@forki

This comment has been minimized.

Show comment
Hide comment
Member

forki commented Aug 24, 2016

@eiriktsarpalis

This comment has been minimized.

Show comment
Hide comment
Member

eiriktsarpalis commented Aug 24, 2016

@forki LGTM

@ErikSchierboom ErikSchierboom changed the title from Allow printing of version number through command-line option [WIP] to Allow printing of version number through command-line option Aug 25, 2016

@ErikSchierboom

This comment has been minimized.

Show comment
Hide comment
@ErikSchierboom

ErikSchierboom Aug 25, 2016

Contributor

I'll remove the [WIP] part from the title then.

Contributor

ErikSchierboom commented Aug 25, 2016

I'll remove the [WIP] part from the title then.

@forki forki merged commit 5a97f96 into fsprojects:master Aug 25, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 25, 2016

Member

thanks!

Member

forki commented Aug 25, 2016

thanks!

@ErikSchierboom

This comment has been minimized.

Show comment
Hide comment
@ErikSchierboom

ErikSchierboom Aug 25, 2016

Contributor

🎉

Contributor

ErikSchierboom commented Aug 25, 2016

🎉

@ErikSchierboom ErikSchierboom deleted the ErikSchierboom:version branch Aug 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment