Skip to content

Conversation

macintacos
Copy link
Contributor

@macintacos macintacos commented Jun 8, 2022

On Go 1.17, go get -u as a method of installation locally was deprecated, and on 1.18 it completely fails to run. Switching to go install github.com/arl/gitmux@latest works as expected on 1.17+.

Purpose

To make the README slightly more accurate depending on what version you have chosen.

Approach

Clearly denotes what command to use depending on what version of Go you decide to use.

On Go 1.17, `go get -u` as a method os installation locally was deprecated, and on 1.18 it completely fails to run. Switching to `go install github.com/arl/gitmux@latest` works as expected on 1.17+.
Copy link
Owner

@arl arl left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, much appreciated!

Since we're way past 1.18 release, and soon 1.19, i'm even wondering if we should just directly give install instructions saying that we need go1.17+ and go install.
What do you think?

@macintacos
Copy link
Contributor Author

Sounds perfectly reasonable, I can adjust. I didn't test this on anything other than Go 1.18, however (did the lookup to find that it was deprecated on 1.17). I don't know if go install works on 1.15 or 1.16. Do you have the means to test that out? Otherwise I can do it, just will take a bit for me to get around to it (this was kind-of just a fly-by PR 🙂)

I guess you could also bump go.mod to 1.17 minimum (that's probably what I would do if I were in your shoes). Let me know what you would like to do!

@arl
Copy link
Owner

arl commented Jun 9, 2022

go install ... has been working since go1.16 (included).
I guess it's totally fine to increase the min version from 1.15 to 1.16 in the README.

I'll take care of go.mod in another PR, i prefer this to just be about the README.
Thank you 🙏

@macintacos macintacos requested a review from arl June 9, 2022 19:55
@macintacos
Copy link
Contributor Author

@arl ready for another look!

Copy link
Owner

@arl arl left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@arl arl merged commit 9f9de9b into arl:main Jun 9, 2022
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