Update to Go 1.20 #454
Update to Go 1.20 #454
Conversation
| --license-type apache \ | ||
| --copyright-holder "Buf Technologies, Inc." \ | ||
| --year-range "$(COPYRIGHT_YEARS)" | ||
| $(BIN)/license-header \ |
There was a problem hiding this comment.
This also updates to the new license-header flow
|
|
||
| $(BIN)/license-header: Makefile | ||
| @mkdir -p $(@D) | ||
| GOBIN=$(abspath $(@D)) $(GO) install \ | ||
| github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.9.0 | ||
| github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@90fa81df0e9ef86ed505956be1ab1e8ccd49aa52 |
There was a problem hiding this comment.
We can update this to a SemVer tag on the next release
|
|
||
| $(BIN)/golangci-lint: Makefile | ||
| @mkdir -p $(@D) | ||
| GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.0 | ||
| GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.0 |
There was a problem hiding this comment.
This is required to get Golang 1.20 to work
| @@ -135,7 +135,8 @@ func TestHandler_ServeHTTP(t *testing.T) { | |||
| assert.Equal(t, resp.StatusCode, http.StatusNotFound) | |||
|
|
|||
| type errorMessage struct { | |||
| Code, Message string | |||
| Code string `json:"code,omitempty"` | |||
There was a problem hiding this comment.
This failed on golangci-lint v1.51.0 otherwise
| @@ -1,6 +1,6 @@ | |||
| module github.com/bufbuild/connect-go | |||
|
|
|||
| go 1.18 | |||
| go 1.19 | |||
There was a problem hiding this comment.
@akshayjshah the PR description says Note that go.mod is not changed from 1.18 as we do not use any 1.19-specific features yet, and want to remain as compatible as possible. but it looks like it got updated?
This inconvenienced a customer: https://bufbuild.slack.com/archives/C03H66632C9/p1676658888902709 so if it wasn't necessary we could consider moving this back to 1.18.
There was a problem hiding this comment.
Whoops, this likely should not have made it through, that looks to be on me
There was a problem hiding this comment.
I didn't update the PR description, so that's my fault. This changeset drops a set of Go 1.18-specific patches and the accompanying build tag complexity.
The compatibility guarantee that this library (and all our other Go libraries) offer is the same as the Go compiler itself - current version + one older one. The customer was using a compiler that's >1y old and is no longer being patched or supported by the Go team.
Do we want to support compilers that the Go team doesn't? If so, for how long?
There was a problem hiding this comment.
Unless there was more complexity removed in subsequent PRs, the complexity in this PR diff doesn't seem like that big of a deal to support (at least for a bit longer). Was it getting in our way?
If we are using Go support as a baseline, I would think we would want to support at least N+1 (e.g. last 3 version of Go) to give customers a buffer (so they can upgrade Go and then upgrade us) since we are downstream of Go.
There was a problem hiding this comment.
Eh I get both trains of thought here. I'm not plussed about having to maintain support for more than 1 version back, but you could make an argument that unless it's problematic, a foundational library like connect-go may strive to maintain further backcompat than you might otherwise. A long way of saying that we might want to maintain 1.18 support until we don't have to. I know @jhump maintains support going back to like go 1.2 in his libraries, as a good counterexample.
There was a problem hiding this comment.
(supporting go 1.2 is a joke, but I know he only updates when he has to)
There was a problem hiding this comment.
Go isn't Java or Swift: as of the last available data, only 3% of Go users take >1y to update to new releases. >50% update within 2 months. (Source is the 2021 Go developer survey. Answers to that question were consistent enough year to year that they've stopped reporting results.) The customer in question was updating to Go 1.19 the very same day they ran into this problem.
Making compiler support a nuanced, ad-hoc decision made in each project has notable downsides: whether a connect-go service builds with a given compiler version becomes an emergent property of the particular libraries in question, rather than a clear policy.
I think it's easy to make a small change that avoids the particular issue we ran into here without preventing us from doing routine housekeeping: drop support for old compilers ~1 month after they're end-of-lifed, rather than ~1w as we did here.
There was a problem hiding this comment.
drop support for old compilers ~1 month after they're end-of-lifed, rather than ~1w as we did here.
I am fine with this as it addresses the grace period problem and may be sufficient in practice.
There was a problem hiding this comment.
I know @jhump maintains support going back to like go 1.2 in his libraries, as a good counterexample.
FWIW, I only provided that kind of support because it was easy to do so (not many new language features that were compelling to adopt). At first, I did it intentionally because FullStory (once upon a time) had code running on the Go runtime for Google AppEngine (which at the time was very out of date from mainstream Go releases).
Even after our migration off of AppEngine, I basically let my protoreflect repo continue to support Go versions for as long as the official protobuf and grpc repos did (so I only removed old versions when those other packages stopped supporting them).
@bufdev, you'll be glad to know that is no longer the case. As of v1.15.0, jhump/protoreflect now requires Go 1.18 or higher 😛
There was a problem hiding this comment.
👋 as a marginal outsider and heaver user, I fully support using latest and latest-1 versions.
I think, if anything, connect-go is a bit more bleeding edge software and under active development. As a consumer of this library, I find it quite a bit more essential to stay up to date, and we're not as likely to be dealing with legacy software that's been shipped in maintenance mode for years.
Maybe in the future there comes a time when there's demand for some more backwards compatibility, but for the time being, I'd vote to stay as latest as we can to adopt newest features with the least amount of backwards compatibility shims for older versions.
We (PlanetScale) will trail behind adopting the latest new major versions of go until the x.x.1 releases usually to ensure some extra level of stability, but other than that, we try to stay on latest as much as possible.
makewas not working with 1.20, this fixes it and upgrades CI to use 1.19 and 1.20. Note thatgo.modis not changed from 1.18 as we do not use any 1.19-specific features yet, and want to remain as compatible as possible. There was one new issue withgolangci-lintthat required JSON tags, so I just fixed that.