chore: keeping project up to date#44
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the project's dependencies, refactors transport layer code, and introduces new development environment tooling. The changes include upgrading Go and various dependencies to future versions (which appear to be from 2026), refactoring WebSocket and gRPC transport implementations to improve code organization, and adding devenv configuration for local development.
Changes:
- Upgraded Go version to 1.24.0 and updated all major dependencies including grpc-gateway to v2
- Refactored WebSocket and gRPC transport code with improved error handling, cleaner interfaces, and better encapsulation
- Added devenv development environment configuration and updated CI/CD workflows
Reviewed changes
Copilot reviewed 34 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod, go.sum | Updated Go to 1.24.0 and upgraded dependencies |
| transport/error_handler.go | New ErrorHandlers type to aggregate error handlers |
| transport/ws/*.go | Refactored upgrader/dialler to use composition, improved connection handling |
| transport/grpc/*.go | Changed receivers from value to pointer, integrated ErrorHandlers |
| test/transport/**/*.go | Updated import aliases and build tags |
| .golangci.yml | Simplified linter configuration |
| devenv.* | Added devenv development environment files |
| .github/workflows/*.yaml | Updated GitHub Actions versions |
| README.md | Minor formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dialer := &dialler{ | ||
| ws: &websocket.Dialer{ | ||
| Proxy: http.ProxyFromEnvironment, | ||
| HandshakeTimeout: 45 * time.Second, | ||
| }, | ||
| } | ||
| for _, f := range c.opts.before { | ||
| ctx = f(ctx, dialer, headers) | ||
| } | ||
|
|
||
| wsc, resp, err := dialer.DialContext(ctx, c.url.String(), headers) | ||
| wsc, resp, err := dialer.ws.DialContext(ctx, c.url.String(), headers) |
There was a problem hiding this comment.
After passing the dialler through the before callbacks (line 60), the code directly accesses the internal ws field (line 63: dialer.ws.DialContext). This breaks encapsulation since the dialler is typed as Dialler interface in the callbacks but then used as the concrete *dialler type. Consider either: 1) adding a DialContext method to the Dialler interface, or 2) using the concrete type throughout instead of the interface.
| version: "2" | ||
|
|
||
| run: | ||
| timeout: 5m | ||
| issues-exit-code: 1 | ||
| tests: false | ||
| modules-download-mode: readonly | ||
| allow-parallel-runners: true | ||
|
|
||
| output: | ||
| print-issued-lines: true | ||
| print-linter-name: true | ||
|
|
||
| linters: | ||
| disable: | ||
| - depguard | ||
| - exhaustruct | ||
| - fatcontext | ||
| - gochecknoglobals | ||
| - goconst | ||
| - godot | ||
| - godox | ||
| - nonamedreturns | ||
| - nlreturn | ||
| - varnamelen | ||
| - inamedparam | ||
| - wsl | ||
| - wrapcheck | ||
| presets: | ||
| - bugs | ||
| - comment | ||
| - complexity | ||
| - error | ||
| - format | ||
| - import | ||
| - metalinter | ||
| - module | ||
| - performance | ||
| - style | ||
| - test | ||
| - unused | ||
|
|
||
| linters-settings: | ||
| exhaustive: | ||
| ignore-enum-members: '^*(Unspecified|Undefined|Unknown|Idle)$' | ||
| cyclop: | ||
| max-complexity: 20 | ||
| gomnd: | ||
| ignored-numbers: | ||
| - '2' | ||
| - '4' | ||
| - '8' | ||
| - '16' | ||
| - '32' | ||
| - '64' | ||
| gci: | ||
| sections: | ||
| - standard # Standard section: captures all standard packages. | ||
| - default # Default section: contains all imports that could not be matched to another section type. | ||
| - prefix(github.com/einouqo/ext-kit) # Custom section: groups all imports with the specified Prefix. | ||
| - blank # Blank section: contains all blank imports. This section is not present unless explicitly enabled. | ||
| - dot | ||
| tagliatelle: | ||
| case: | ||
| rules: | ||
| json: snake | ||
| yaml: snake | ||
| exclusions: | ||
| rules: | ||
| # an exception, as the error value is used to signal the end of the stream | ||
| - linters: | ||
| - staticcheck | ||
| source: "StreamDone" | ||
| # as documentation states: "... This method always returns a nil error" https://github.com/grpc/grpc-go/blob/v1.78.0/stream.go#L105-L108 | ||
| - linters: | ||
| - errcheck | ||
| source: "defer .*\\.CloseSend()" | ||
|
|
There was a problem hiding this comment.
The .golangci.yml config has been dramatically simplified from a comprehensive configuration to minimal exclusions. While this may be intentional, it removes all linter presets, custom settings, and most exclusion rules that were previously in place. This could lead to many new linting errors appearing. Consider whether this level of simplification is intended, or if key configurations should be retained.
| type Upgrader interface { | ||
| SetHandshakeTimeout(timeout time.Duration) | ||
| SetReadBufferSize(size int) | ||
| SetWriteBufferSize(size int) | ||
| SetWriteBufferPool(pool websocket.BufferPool) | ||
| SetHandshakeTimeout(time.Duration) | ||
| SetReadBufferSize(int) | ||
| SetWriteBufferSize(int) | ||
| SetWriteBufferPool(BufferPool) | ||
| SetSubprotocols(protocols []string) | ||
| SetErrorWriter(writer func(w http.ResponseWriter, r *http.Request, status int, reason error)) | ||
| SetCheckOrigin(check func(r *http.Request) bool) | ||
| SetErrorWriter(ErrorWriter) | ||
| SetCheckOrigin(check CheckOrigin) | ||
| SetEnableCompression(enabled bool) | ||
| } |
There was a problem hiding this comment.
The Upgrader interface is missing an Upgrade method, but the upgrader implementation has one (line 39-41) and it's being used in server.go line 75. The interface should include: Upgrade(w http.ResponseWriter, r *http.Request, responseHeader http.Header) (*websocket.Conn, error) to match the implementation.
No description provided.