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

nextercism: implement version command #412

Closed
kytrinyx opened this issue Aug 4, 2017 · 13 comments
Closed

nextercism: implement version command #412

kytrinyx opened this issue Aug 4, 2017 · 13 comments

Comments

@kytrinyx
Copy link
Member

kytrinyx commented Aug 4, 2017

To be merged into the nextercism branch in #410

Command name: version (alias v).
File: cmd/version.go.

Current Version

The default behavior of this command is to output the current version.

Latest Version

When passed the --latest (-l) flag, it checks whether or not there's a newer version available.

We have logic for this in the cli package: https://github.com/exercism/cli/blob/master/cli/cli.go#L66

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 4, 2017

@nywilken You mentioned being interested in getting involved with the new CLI implementation. This issue should be a good first issue to get started with, to help you get your head wrapped around the project.

To work on it, pull down and branch off of the nextercism branch on this repository. That's where you would submit the PR back to (you can choose the "base" branch when creating the PR. If you get it wrong—I almost always do—then you can edit the PR to make it go against the correct branch after).

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 4, 2017

Oh, and to set the flag on a command you would stick this in the init() for the command:

versionCmd.Flags().BoolP("latest", "l", false, "something that describes the flag")

@nywilken
Copy link
Contributor

nywilken commented Aug 4, 2017

Sweet! Thanks for throwing this my way. I will dive in this weekend and reach out if I run into any issues or have questions.

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 4, 2017

Awesome! I'm sticking good-first-patch labels on a few other things as well, and then as we dig into it we'll figure out what other pieces would be good to carve off.

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 7, 2017

Did you get a chance to dive in at all? (No pressure, just curious.)

@nywilken
Copy link
Contributor

nywilken commented Aug 7, 2017

Not yet, I spent some time looking through the PRs you asked for my review to get a sense of how things are currently working. I planned to attack this tonight and tomorrow. If that's okay?

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 7, 2017

✨ Excellent. Do holler if you run into anything, I plan to be around and working on this both tonight and tomorrow.

@nywilken
Copy link
Contributor

nywilken commented Aug 7, 2017

Sounds good, thanks.

@nywilken
Copy link
Contributor

nywilken commented Aug 8, 2017

@kytrinyx any particular reason for using BoolP over BoolVarP?

versionCmd.Flags().BoolP("latest", "l", false, "something that describes the flag")

Versus

versionCmd.Flags().BooVarlP(&checkLatest, "latest", "l", false, "something that describes the flag")

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 8, 2017

No, not in this case. With other flags I load them into viper and then a struct, so the var didn't make sense there, but in this case a variable would be better.

@nywilken
Copy link
Contributor

nywilken commented Aug 8, 2017

Ah okay, that makes sense. Thanks for the quick response. I'll should have a PR for this tomorrow.

Regarding the messaging, was something formalized within a discussion? Or would something like the following be okay?

> exercism v -l
exercism version X.Y.Z
Your CLI [is up to date|does not match the latest version A.B.C].

Maybe even some text calling out the upgrade command if current version is not newer than latest.

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 8, 2017

We haven't worked out the messaging yet. I'm going to do a full pass on messaging at the end when we see what the scope of it is.

I think it makes sense to call out the upgrade command if a newer version is available.

@kytrinyx
Copy link
Member Author

Closed by #432

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

No branches or pull requests

2 participants