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

Add -k/--known to list known Go versions #100

Merged
merged 5 commits into from
Jul 8, 2017

Conversation

ellotheth
Copy link
Contributor

I found myself visiting the Go release notes page to see which patch releases were available for specific releases. This PR makes that list available with gimme -k.

I used https://golang.org/dl as the base list URL instead of GIMME_DOWNLOAD_BASE because the latter list doesn't include the (current) 1.9 beta. The golang.org list doesn't include old betas or release candidates, so maybe a combination of the two would be a better approach?

@philpennock
Copy link
Contributor

I like it. One nit and one open question, neither of which blocks merge IMO but the question is around behavior where we wouldn't want to bumble around post-merge so if we change then defer merge.

nit: | grep . to get rid of the blank line in the output (or use arrays in the first place), so that we only output lines with version numbers on -- this is a nicer API to offer for scripting.

question: what should happen about locally-installed-but-not-available versions? This PR declares known to be "what could be installed if not already installed locally", but old betas from previous versions might be available for activation. So getting a list of files from ${GIMME_ENV_PREFIX} and matching via regexp against those, adding to the list, and adding -u to the sort to uniq would augment the list of "known".

So as it stands, this is "installable", not "known". Options are (1) we're totally happy with that, merge as is; (2) we're happy with behavior, rename the option and merge; (3) augment the behavior to merge installed versions first.

@meatballhat and @ellotheth ? Thoughts?

And @ellotheth if we want changes let us know if you're happy making them yourself, or if you'd rather they be done separately. I like the changed behavior and would be happy to pick up, but don't want to barge in unwanted.

@ellotheth
Copy link
Contributor Author

@philpennock Thanks for the review and feedback!

nit: | grep . to get rid of the blank line in the output

👍

what should happen about locally-installed-but-not-available versions?

You're right, known should mean more than versions available on the official site. Can I cheat and add the existing _list_versions output to _list_known?

@philpennock philpennock merged commit b56eeef into travis-ci:master Jul 8, 2017
@philpennock
Copy link
Contributor

Thanks!

@ellotheth
Copy link
Contributor Author

🎉 Most welcome!

@ellotheth ellotheth deleted the list-known branch July 10, 2017 14:59
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