Repository navigation
Conversation
Upgrade the module and CI to Go 1.27.2, apply modernizers to a fixpoint, and migrate lint tooling. Pin workflow actions to verified stable release commits. Validated build, vet, tests, race tests, lint, formatting, and actionlint.
tmaxmax
left a comment
tmaxmax
left a comment
There was a problem hiding this comment.
we should also add an ## Unreleased section to the changelog which should document that the minimum supported version will be 1.26.
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 |
There was a problem hiding this comment.
any reason for not using just the version, e.g. actions/setup-go@v7?
| @@ -1,3 +1,3 @@ | |||
| module github.com/tmaxmax/go-sse | |||
|
|
|||
| go 1.22 | |||
| sctx, cancel := context.WithTimeout(context.Background(), time.Second*10) | ||
| sctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Second*10) |
There was a problem hiding this comment.
Unnecessary since the Shutdown call gives no opportunity for using context values.
| @@ -37,11 +37,9 @@ func newSSE() *sse.Server { | |||
| topics = r.URL.Query()["topic"] | |||
| for _, topic := range topics { | |||
| if topic != topicRandomNumbers && topic != topicMetrics { | |||
There was a problem hiding this comment.
Let's not clutter this PR with unrelated example code changes.
| test-old: | ||
| name: Test (previous major) |
| 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. |
There was a problem hiding this comment.
We shouldn't change the support promise.
| return true // or false to stop iteration | ||
| }) | ||
| ``` | ||
| This feature uses Go's [range-over-function iterators](https://go.dev/blog/range-functions). |
There was a problem hiding this comment.
Can remove this line fully, range-over-func is normal today.
| } | ||
|
|
||
| if err != nil && !(ignoreEOF && isEOF) { | ||
| if err != nil && (!ignoreEOF || !isEOF) { |
| - name: Check formatting | ||
| run: golangci-lint fmt --diff |
There was a problem hiding this comment.
Not needed. golangci-lint run, executed by default, already checks formatting.
| pub := publishedMessage{replayerErr: errs} | ||
| pub.message = msg | ||
| pub.topics = topics | ||
| pub := publishedMessage{ | ||
| replayerErr: errs, | ||
| message: msg, | ||
| topics: topics, | ||
| } |
There was a problem hiding this comment.
Since for now go1.26 is the intended minimum target we should remove this class of changes.
|
As part of this PR, could you also remove the Go report badge from README? |
Upgrade the module and CI to Go 1.27.2, apply modernizers to a fixpoint, and migrate lint tooling. Pin workflow actions to verified stable release commits.
Validated build, vet, tests, race tests, lint, formatting, and actionlint.