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.

Sign up for GitHub

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

Jump to bottom

Bump Go version to 1.22 #174

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Bump Go version to 1.22 #174

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a3ea2fa
Bump go to v1.22
emcfarlane Feb 22, 2024
75b2dd4
Bump LICENSE
emcfarlane Feb 22, 2024
4720014
Fix lint
emcfarlane Feb 22, 2024
7594a8b
Fix test assert
emcfarlane Feb 22, 2024
d4b1031
Update ci.yaml
emcfarlane Apr 18, 2024
f586cb5
Revert to go1.22 ci.yaml
emcfarlane Apr 18, 2024
298c371
Update to go1.21.x ci.yaml
emcfarlane Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- name: setup-go
uses: actions/setup-go@v5
with:
go-version: 1.21.x
go-version: 1.22.x
Copy link
Member

Choose a reason for hiding this comment

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

If go.mod states Go 1.20, and we're not using a matrix here to run these steps across multiple versions, seems like we should actually set this to 1.20.x instead of updating it to 1.22. Why should this be using latest?

Copy link
Author

Choose a reason for hiding this comment

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

Was following current, suppose its better to test old versions of go then latest. Have set at 1.20.x.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted, lint breaks and is sensitive to go version. We could create a matrix for these test cases but I think it's safe for example code to be run at the latest version.

Copy link
Member

@jhump jhump Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

safe for example code to be run at the latest version

Then we should change go.mod to 1.22, or else we're not actually verifying that the repo works, like for people that pull down the contents and use an earlier (ostensibly) supported version.

I think it's better to just leave this at the older version. Or we could move it to Go 1.21, since that is the oldest Go version still getting patches.

lint breaks and is sensitive to go version

Shouldn't it be sensitive to the version of golangci-lint, not the version of go? Or if there's a breakage due to gofmt changes from 1.20 to 1.22, we can just format it with 1.20 for now.

- name: make-test
run: make test && make checkgenerate
- name: make-lint
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
same "printed page" as the copyright notice for easier
identification within third-party archives.

Copyright 2022 The Connect Authors
Copyright 2022-2024 The Connect Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
8 changes: 4 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ MAKEFLAGS += --no-print-directory
BIN=$(abspath .tmp/bin)
export PATH := $(BIN):$(PATH)
export GOBIN := $(abspath $(BIN))
COPYRIGHT_YEARS := 2022-2023
COPYRIGHT_YEARS := 2022-2024
LICENSE_IGNORE := --ignore /testdata/
# Set to use a different compiler. For example, `GO=go1.18rc1 make test`.
GO ?= go
Expand Down Expand Up @@ -68,15 +68,15 @@ checkgenerate:

$(BIN)/buf: Makefile
@mkdir -p $(@D)
$(GO) install github.com/bufbuild/buf/cmd/buf@v1.26.1
$(GO) install github.com/bufbuild/buf/cmd/buf@v1.29.0

$(BIN)/license-header: Makefile
@mkdir -p $(@D)
$(GO) install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.26.1
$(GO) install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.29.0

$(BIN)/golangci-lint: Makefile
@mkdir -p $(@D)
$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.1
$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56.2

$(BIN)/protoc-gen-go: Makefile
@mkdir -p $(@D)
Expand Down
10 changes: 5 additions & 5 deletions cmd/demoserver/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -53,7 +53,7 @@ func (e *elizaServer) Say(
_ context.Context,
req *connect.Request[elizav1.SayRequest],
) (*connect.Response[elizav1.SayResponse], error) {
reply, _ := eliza.Reply(req.Msg.Sentence) // ignore end-of-conversation detection
reply, _ := eliza.Reply(req.Msg.GetSentence()) // ignore end-of-conversation detection
return connect.NewResponse(&elizav1.SayResponse{
Sentence: reply,
}), nil
Expand All @@ -73,7 +73,7 @@ func (e *elizaServer) Converse(
} else if err != nil {
return fmt.Errorf("receive request: %w", err)
}
reply, endSession := eliza.Reply(request.Sentence)
reply, endSession := eliza.Reply(request.GetSentence())
if err := stream.Send(&elizav1.ConverseResponse{Sentence: reply}); err != nil {
return fmt.Errorf("send response: %w", err)
}
Expand All @@ -88,7 +88,7 @@ func (e *elizaServer) Introduce(
req *connect.Request[elizav1.IntroduceRequest],
stream *connect.ServerStream[elizav1.IntroduceResponse],
) error {
name := req.Msg.Name
name := req.Msg.GetName()
if name == "" {
name = "Anonymous User"
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func newCORS() *cors.Cors {
http.MethodPatch,
http.MethodDelete,
},
AllowOriginFunc: func(origin string) bool {
AllowOriginFunc: func(_ string) bool {
// Allow all origins, which effectively disables CORS.
return true
},
Expand Down
31 changes: 17 additions & 14 deletions cmd/demoserver/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -17,7 +17,6 @@ package main
import (
"context"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -59,8 +58,8 @@ func TestElizaServer(t *testing.T) {
result, err := client.Say(context.Background(), connect.NewRequest(&elizav1.SayRequest{
Sentence: "Hello",
}))
require.Nil(t, err)
assert.True(t, len(result.Msg.Sentence) > 0)
require.NoError(t, err)
assert.NotEmpty(t, result.Msg.GetSentence())
}
})
t.Run("converse", func(t *testing.T) {
Expand All @@ -74,9 +73,11 @@ func TestElizaServer(t *testing.T) {
defer wg.Done()
for _, sentence := range sendValues {
err := stream.Send(&elizav1.ConverseRequest{Sentence: sentence})
require.Nil(t, err, fmt.Sprintf(`failed for string sentence: "%s"`, sentence))
if !assert.NoError(t, err, `failed for string sentence: "%s"`, sentence) {
break
}
}
require.Nil(t, stream.CloseRequest())
assert.NoError(t, stream.CloseRequest())
}()
go func() {
defer wg.Done()
Expand All @@ -85,11 +86,13 @@ func TestElizaServer(t *testing.T) {
if errors.Is(err, io.EOF) {
break
}
require.Nil(t, err)
assert.True(t, len(msg.Sentence) > 0)
receivedValues = append(receivedValues, msg.Sentence)
if !assert.NoError(t, err) {
break
}
assert.NotEmpty(t, msg.GetSentence())
receivedValues = append(receivedValues, msg.GetSentence())
}
require.Nil(t, stream.CloseResponse())
assert.NoError(t, stream.CloseResponse())
}()
wg.Wait()
assert.Equal(t, len(receivedValues), len(sendValues))
Expand All @@ -102,13 +105,13 @@ func TestElizaServer(t *testing.T) {
Name: "Ringo",
})
stream, err := client.Introduce(context.Background(), request)
assert.Nil(t, err)
require.NoError(t, err)
for stream.Receive() {
total++
}
assert.Nil(t, stream.Err())
assert.Nil(t, stream.Close())
assert.True(t, total > 0)
require.NoError(t, stream.Err())
require.NoError(t, stream.Close())
assert.Greater(t, total, 0)
}
})
}
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Users should clone the repo to explore the examples.
module connect-examples-go

go 1.18
go 1.20

require (
connectrpc.com/connect v1.14.0
Expand Down
2 changes: 1 addition & 1 deletion internal/eliza/eliza.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion internal/eliza/eliza_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion internal/eliza/globals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion internal/gen/connectrpc/eliza/v1/eliza.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/gen/connectrpc/eliza/v1/elizav1connect/eliza.connect.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion proto/connectrpc/eliza/v1/eliza.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 The Connect Authors
// Copyright 2022-2024 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down
Loading