Skip to content

chore: Bump go version to 1.14 in go.mod #1210

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

Closed
wants to merge 1 commit into from

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Jul 5, 2020

Golang 1.12 is EOL as per https://golang.org/doc/devel/release.html

Signed-off-by: Tam Mach [email protected]

@sayboras sayboras requested a review from a team July 5, 2020 10:07
@Helcaraxan
Copy link
Contributor

I am not entirely sure of the intention behind the change. The go directive in the go.mod has nothing to do with the version of the actual toolchain that is used to build the project. It's simply an indication of the language features that the project relies on. It means that if you use, for example, binary int constants while setting go 1.12 and compile with Go 1.14.4 it will throw an error because these type of constants were not supported in 1.12.

With this change it means that we could start using the newer features if we want to but anyone who is compiling the project from source with an older toolchain might be in trouble. If there are no particular newer features that we want to use there's no reason to change the directive.

@sayboras
Copy link
Member Author

sayboras commented Jul 5, 2020

I am not entirely sure of the intention behind the change. The go directive in the go.mod has nothing to do with the version of the actual toolchain that is used to build the project. It's simply an indication of the language features that the project relies on. It means that if you use, for example, binary int constants while setting go 1.12 and compile with Go 1.14.4 it will throw an error because these type of constants were not supported in 1.12.

Yes, you are right. I agreed that this version is having nothing to do with version of toolchain. Its just merely to avoid confusion that the project supports for golang 1.12 build and related stuffs. Consider that we have our CI running test on latest go 1.13 and go.14, I don't think we still support 1.12.

With this change it means that we could start using the newer features if we want to but anyone who is compiling the project from source with an older toolchain might be in trouble. If there are no particular newer features that we want to use there's no reason to change the directive.

It was a little agressive to make it 1.14. I can just move to 1.13 as per the official website https://golangci-lint.run/contributing/workflow/. What do you think ?

@Helcaraxan
Copy link
Contributor

Helcaraxan commented Jul 5, 2020

It think the question is rather why we "require" 1.13+ in the contribution docs. As far as I know one can perfectly use 1.12. The CI will run on the newer versions (as it should).

And even beyond that, even if we require 1.13+ for some reason (probably related to fixes in the toolchain w.r.t modules) then that still doesn't mean that we need or should upgrade the go directive. If we never adopt newer language features than we could end up developing with Go 1.20 in 3 years time and the 1.12 directive would still be OK.

@sayboras
Copy link
Member Author

sayboras commented Jul 5, 2020

@Helcaraxan I think I got where you are coming from. I don't have golang 1.12 along with related tool chain (compile with golang 1.12) to test it out. But having a quick scan on code base, seem like golangci-lint will be just fine with 1.12.

For contributition docs mentioning 1.13, maybe just to be safe that way, and I think we can just let it be. Majority of new contributor (like myself) will problably have go 1.13+ anyway.

Closing this PR, we can anyway open again if required. Thanks for chiming in @Helcaraxan 💯

@sayboras sayboras closed this Jul 5, 2020
@sayboras sayboras deleted the chore/bump-go-version-mod branch July 5, 2020 10:48
@Helcaraxan
Copy link
Contributor

Yes. If we need to update the directive that is perfectly fine by me. It was as just that in my eyes the provided reason was not a sufficient one. 🙂

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