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

Argument to version command - path or dependency name? #226

Open
straight-shoota opened this issue Sep 6, 2018 · 6 comments
Open

Argument to version command - path or dependency name? #226

straight-shoota opened this issue Sep 6, 2018 · 6 comments

Comments

@straight-shoota
Copy link
Member

#148 introduced shards version command which prints the version specified in shard.yml. The command accepts an optional parameter path which can be used to specifiy to look for shard.yml in a different path.

@luislavena Was there a specific reason for this feature?

To me, it feels a little bit strange. I mean, it being able to specify a different directory is nice, but wouldn't that make sense for all commands? Similar to make -C path. I think it is better to use a named option. It is not intuitive that the first argument to version is interpreted as a path to look for a shard.yml.

I'd rather suggest shards version to have an argument to specify a dependency name and it would print the installed version of that dependency.
For example, inside shards directory, shards version minitest would print 0.3.5.

@luislavena
Copy link
Contributor

Hello @straight-shoota, the reasoning for the second argument is explained in the pull request, which is a response to the RFC #147.

See #147 (comment) for the specific use case.

@straight-shoota
Copy link
Member Author

Ah, somehow I missed that. Thanks for pointing it out.

The example in the linked comment should also work with shards version -C __DIR__, for example. Or maybe just shards version wonderful which would resolve the location of the shard (wonderful in this example). In this use case, specifying the shard name has a little bit overhead because it resolves the location again which is already available in __DIR__. But considering that it needs to traverse the parent directory, this doesn't really matter.

@ysbaddaden
Copy link
Contributor

I suppose we could have shards version [-c <path>] [<dependency>]. It's a breaking change, but the command would feel a little more complete.

@straight-shoota
Copy link
Member Author

straight-shoota commented Sep 6, 2018

For backwards compatibility, it could even treat <dependecy> as a path if no dependency with that name is specified (and the path exists, obviously).

@bcardiff
Copy link
Member

Since there has been no PR for this issue so far, I will remove it from the milestone v0.10.0

@bcardiff bcardiff removed this from the v0.10.0 milestone Mar 27, 2020
@straight-shoota
Copy link
Member Author

I would suggest to close this issue in favour of a more general approach to querying information (including the version) of installed shards (see #86 (comment)).

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

No branches or pull requests

4 participants