Skip to content
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

Explicitly state build_options in help output, fixes #363 #364

Merged

Conversation

Darwinnn
Copy link
Contributor

The [<options>] in the build argument was a bit misleading, it's more clear now on what options to use with build

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

Could you also update the manpage to use the terms build_options and GENERAL OPTIONS?

src/cli.cr Outdated
version [<path>] - Print the current version of the shard.

Build options:
Use "crystal build --help" to see available build options.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should be mentioned here. There are no explanations for other command's arguments either.
That would better fit for a more detailled shards build --help if that was a thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since shards build --help is not a thing, I'm assuming we have to somehow clarify that build_options have nothing to do with GENERAL OPTIONS, same way it's stated in the manpage.

I also don't see a way how we could implement shards build --help without copying the info from crystal build --help directly and then somehow keeping these two in sync.

Any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

Well shards build --help wouldn't list the options explicitly but show more details information about the command itself, just like the build entry on the manpage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, we can specify it in the build arg itself, does this seem suitable?

build [<targets>] [<build_options>]  - Build the specified <targets> in `bin` path, all build_options are delegated to `crystal build`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @RX14! I've updated the code accordingly.

@Darwinnn
Copy link
Contributor Author

Could you also update the manpage to use the terms build_options and GENERAL OPTIONS?

Oh I missed that, thanks! Updated.

@Darwinnn Darwinnn force-pushed the fix/363/help_output_on_build_options branch from 928a174 to 5a039fa Compare April 25, 2020 21:09
@waj waj linked an issue Apr 28, 2020 that may be closed by this pull request
@waj waj merged commit 3387c00 into crystal-lang:master Apr 28, 2020
@bcardiff bcardiff added this to the v0.11.0 milestone May 27, 2020
taylor pushed a commit to vulk/shards that referenced this pull request Aug 11, 2020
f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021
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.

shards build in release/production mode
5 participants