search

google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.26k stars 204 forks source link

Bump go version to 1.19 #426

Closed ahrtr closed 1 year ago

ahrtr commented 1 year ago

The pipeline is already using go1.18 and 1.19, so we'd better to bump go version in go.mod file as well.

I intentionally created three different commits, see detailed info below.

Steps what I did:

  1. Manually updated go version to 1.19 in the go.mod file; See https://github.com/google/starlark-go/commit/a9d10e312664bb26fae99b0803e189555c981e7a
  2. Format all go source file by just executing "gofmt -w .". See https://github.com/google/starlark-go/commit/591514d7356a5c3655b02e985b65dfdac248e4a1
  3. Update all dependencies by just executing "go mod tidy". See https://github.com/google/starlark-go/commit/64f0299a4ab1f5edf81a940824d54ef85793fe16
adonovan commented 1 year ago

I'm not sure what you mean by "the pipeline", but we shouldn't drop support for go1.18 (or even go1.17) yet. This will have to wait for another cycle. Sorry.

ahrtr commented 1 year ago

I'm not sure what you mean by "the pipeline", but we shouldn't drop support for go1.18 (or even go1.17) yet. This will have to wait for another cycle. Sorry.

Thanks for the response. Points from my side,

  1. pipeline means github workflow, and the version is 1.18 and 1.19
  2. Bumping go version to 1.19 just means developers & contributors need to setup dev environment using go 1.19. It doesn't mean to drop support for any version. As long as the project continue to test against 1.18 and 1.19, then it means that it supports 1.18 and 1.19 at least.
adonovan commented 1 year ago

Bumping go version to 1.19 just means developers & contributors need to setup dev environment using go 1.19. It doesn't mean to drop support for any version. As long as the project continue to test against 1.18 and 1.19, then it means that it supports 1.18 and 1.19 at least.

The go version in the go.mod file states the minimum version of Go required by the package and enables certain features of the go build system. In fact, the tools try to be flexible by proceeding to execute a build even when the tool version is lower than the go.mod version, deferring the version error until some compilation fails (for any reason). If the project doesn't use, say, generics, it would build fine within a go1.19 mod file and a go1.18 compiler, and our CI workflow's test of go1.18 effectively ensures that we don't introduce go1.19 features. So you are right that updating it wouldn't fundamentally affect our users.

But what then is the value of upgrading the go declaration to go1.19 if we don't actually require go1.19?

(I was wrong about go1.17: we dropped it as part of #398.)

ahrtr commented 1 year ago

Thanks for the response.

But what then is the value of upgrading the go declaration to go1.19 if we don't actually require go1.19?

If you introduce more workflow test/check (e.g. gofmt), then you will find different versions have different gofmt results. The golangci also reads the go version from go.mod by default.

go mod tidy also has different results if you set different go version in go.mod.

So it would be better to clearly set a go version in go.mod, and clearly requires all contributors setup the dev environment using the same go version.

adonovan commented 1 year ago

Thanks for the response.

But what then is the value of upgrading the go declaration to go1.19 if we don't actually require go1.19?

If you introduce more workflow test/check (e.g. gofmt), then you will find different versions have different gofmt results. The golangci also reads the go version from go.mod by default.

In effect this proposes to add an unnecessary minimum version constraint to all users of this package in order to slightly reduce the burden on the developers of the package, who might forget to test with the latest Go toolchain. I don't think that's the right trade-off, as there are many more of the former than the latter.

ahrtr commented 1 year ago

I think we need to clearly differentiate two roles: users of this packageand contributors of this package. Note that there isn't a minimum version constraint to all users, they can use whatever go version in their applications which depend on starlark-go.

For contributors, I already explained the reasons in my previous comment.

go 1.13 is out of support for a long time, it's not a good pattern to always pin a old & unsupported go version in go.mod.

ahrtr commented 1 year ago

Closing this ticket as I do not have bandwidth to follow up it.