Repository navigation
Modernize for Go 1.27.2 and pin GitHub Actions #62
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,35 +16,30 @@ jobs: | |
| lint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: golangci/golangci-lint-action@v2 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 | ||
| with: | ||
| go-version-file: go.mod | ||
| - uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 | ||
| with: | ||
| version: v2.14.0 | ||
| - name: Check formatting | ||
| run: golangci-lint fmt --diff | ||
| test: | ||
| name: Test (latest major) | ||
| name: Test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-go@v4 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 | ||
|
Comment on lines
+32
to
+33
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason for not using just the version, e.g. |
||
| with: | ||
| go-version: "~1.23.4" | ||
| go-version-file: go.mod | ||
| - name: Test | ||
| run: go test -v -timeout=1s -coverprofile=coverage.txt -covermode=atomic ./... | ||
| - name: Test (race) | ||
| run: go test -v -timeout=1s -race ./... | ||
| - name: Coverage | ||
| uses: codecov/codecov-action@v3 | ||
| if: github.ref == 'refs/head/master' | ||
| uses: codecov/codecov-action@303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1 | ||
| if: github.ref == 'refs/heads/master' | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| files: ./coverage.txt | ||
| test-old: | ||
| name: Test (previous major) | ||
|
Comment on lines
-39
to
-40
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should not remove. |
||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: "~1.22.10" | ||
| - name: Test | ||
| run: go test -v -timeout=1s ./... | ||
| - name: Test (race) | ||
| run: go test -v -timeout=1s -race ./... | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,58 +1,71 @@ | ||
| version: "2" | ||
| linters: | ||
| enable: | ||
| - errcheck | ||
| - gosimple | ||
| - govet | ||
| - ineffassign | ||
| - staticcheck | ||
| - typecheck | ||
| - dogsled | ||
| - dupl | ||
| - errorlint | ||
| - exhaustive | ||
| - nestif | ||
| - goconst | ||
| - gocritic | ||
| - gocyclo | ||
| - godot | ||
| - godox | ||
| - gofmt | ||
| - gofumpt | ||
| - goheader | ||
| - goimports | ||
| - gomoddirectives | ||
| - gomodguard | ||
| - gomodguard_v2 | ||
| - gosec | ||
| - importas | ||
| - makezero | ||
| - misspell | ||
| - nestif | ||
| - nolintlint | ||
| - prealloc | ||
| - promlinter | ||
| - predeclared | ||
| - nolintlint | ||
| - promlinter | ||
| - revive | ||
| - stylecheck | ||
| - staticcheck | ||
| - tagliatelle | ||
| - thelper | ||
| - unparam | ||
| - unused | ||
| - whitespace | ||
| linters-settings: | ||
| gosec: | ||
| excludes: | ||
| - G404 | ||
| gocritic: | ||
| disabled-checks: | ||
| - ifElseChain | ||
| - unnamedResult | ||
| - hugeParam | ||
| enabled-tags: | ||
| - performance | ||
| - diagnostic | ||
| - experimental | ||
| - opinionated | ||
| nestif: | ||
| min-complexity: 8 | ||
| govet: | ||
| enable: | ||
| - fieldalignment | ||
| settings: | ||
| gocritic: | ||
| disabled-checks: | ||
| - ifElseChain | ||
| - unnamedResult | ||
| - hugeParam | ||
| enabled-tags: | ||
| - performance | ||
| - diagnostic | ||
| - experimental | ||
| - opinionated | ||
| gosec: | ||
| excludes: | ||
| - G404 | ||
| govet: | ||
| enable: | ||
| - fieldalignment | ||
| nestif: | ||
| min-complexity: 8 | ||
| exclusions: | ||
| generated: lax | ||
| presets: | ||
| - comments | ||
| - common-false-positives | ||
| - legacy | ||
| - std-error-handling | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
| formatters: | ||
| enable: | ||
| - gofmt | ||
| - gofumpt | ||
| - goimports | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ The library provides both server-side and client-side implementations of the pro | |
|
|
||
| If you are not familiar with the protocol or not sure how it works, read [MDN's guide for using server-sent events](https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events). [The spec](https://html.spec.whatwg.org/multipage/server-sent-events.html) is also useful read! | ||
|
|
||
| `go-sse` promises to support the [Go versions supported by the Go team](https://go.dev/doc/devel/release#policy) – that is, the 2 most recent major releases. | ||
| `go-sse` requires Go 1.27.2 or later. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't change the support promise. |
||
|
|
||
| ## Cut to the chase – how do I read my LLM's response? | ||
|
|
||
|
|
@@ -70,13 +70,7 @@ for ev, err := range sse.Read(res.Body, nil) { | |
|
|
||
| See the [LLM example](cmd/llm/main.go) for a fully working Go program. | ||
|
|
||
| Go 1.23 iterators (officially ["range-over-func"](https://go.dev/blog/range-functions)) are used for this feature. If you are still on Go 1.22 use the `GOEXPERIMENT=rangefunc` environment variable (e.g. `GOEXPERIMENT=rangefunc go run main.go`) or use the iterator without the syntactic sugar: | ||
| ```go | ||
| events(func(ev Event) bool { | ||
| // do something with event | ||
| return true // or false to stop iteration | ||
| }) | ||
| ``` | ||
| This feature uses Go's [range-over-function iterators](https://go.dev/blog/range-functions). | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can remove this line fully, range-over-func is normal today. |
||
|
|
||
| `sse.Read` is also useful if you're implementing an LLM SDK: call it in your code and spare yourself time and maintenance burden by not reimplementing event stream parsing. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,11 +37,9 @@ func newSSE() *sse.Server { | |
| topics = r.URL.Query()["topic"] | ||
| for _, topic := range topics { | ||
| if topic != topicRandomNumbers && topic != topicMetrics { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not clutter this PR with unrelated example code changes. |
||
| fmt.Fprintf(w, "invalid topic %q; supported are %q, %q", topic, topicRandomNumbers, topicMetrics) | ||
|
|
||
| // NOTE: if you are returning false to reject the subscription, we strongly recommend writing | ||
| // your own response code. Clients will receive a 200 code otherwise, which may be confusing. | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| http.Error(w, fmt.Sprintf("invalid topic %q; supported are %q, %q", topic, topicRandomNumbers, topicMetrics), http.StatusBadRequest) | ||
| return nil, false | ||
| } | ||
| } | ||
|
|
@@ -161,7 +159,7 @@ func runServer(ctx context.Context, s *http.Server) error { | |
| go func() { | ||
| <-ctx.Done() | ||
|
|
||
| sctx, cancel := context.WithTimeout(context.Background(), time.Second*10) | ||
| sctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Second*10) | ||
|
Comment on lines
-164
to
+162
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary since the |
||
| defer cancel() | ||
|
|
||
| shutdownError <- s.Shutdown(sctx) | ||
|
|
@@ -178,7 +176,7 @@ func generateRandomNumbers() *sse.Message { | |
| e := &sse.Message{} | ||
| count := 1 + rand.Intn(5) | ||
|
|
||
| for i := 0; i < count; i++ { | ||
| for range count { | ||
| e.AppendData(strconv.FormatUint(rand.Uint64(), 10)) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| //go:build go1.23 | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,7 +126,7 @@ func read(pf func() *parser.Parser, lastEventID string, onRetry func(int64), ign | |
| } | ||
| } | ||
|
|
||
| if err != nil && !(ignoreEOF && isEOF) { | ||
| if err != nil && (!ignoreEOF || !isEOF) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated, let's not change. |
||
| yield(Event{}, err) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| module github.com/tmaxmax/go-sse | ||
|
|
||
| go 1.22 | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
| go 1.27.2 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,9 +171,11 @@ func (j *Joe) Publish(msg *Message, topics []string) error { | |
| // message published causes an error after the shutdown. | ||
| errs := make(chan error, 1) | ||
|
|
||
| pub := publishedMessage{replayerErr: errs} | ||
| pub.message = msg | ||
| pub.topics = topics | ||
| pub := publishedMessage{ | ||
| replayerErr: errs, | ||
| message: msg, | ||
| topics: topics, | ||
| } | ||
|
Comment on lines
-174
to
+178
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since for now go1.26 is the intended minimum target we should remove this class of changes. |
||
|
|
||
| // Waiting on done ensures Publish doesn't block the caller goroutine | ||
| // when Joe is stopped and implements the required Provider behavior. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed.
golangci-lint run, executed by default, already checks formatting.