fix: add default value of WithCustomMessage#19
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
This pull request adds a new configuration option WithCustomMessage to allow users to customize the log message format for HTTP requests. The PR includes the field definition in the Config struct, sets its default value to nil, implements its usage in the logging middleware, and updates the README documentation.
Changes:
- Added
WithCustomMessagefield to Config struct with a function signature that accepts a ResponseWriter and Request to generate custom log messages - Initialized the default value to nil in
DefaultConfig() - Modified the logging logic to use the custom message function when provided
- Added unrelated
fail-fast: falseto GitHub Actions workflow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| middleware.go | Added WithCustomMessage field to Config struct, set default to nil, and implemented custom message logic in the logging middleware |
| README.md | Updated Config struct documentation to include the new WithCustomMessage field |
| .github/workflows/test.yml | Added fail-fast: false to test strategy (unrelated to main PR purpose) |
Comments suppressed due to low confidence (1)
README.md:115
- Consider adding a usage example for the WithCustomMessage feature in the README, similar to the examples provided for other configuration options (OTEL, Custom log levels, Verbose, Filters). This would help users understand how to use the new feature effectively, especially since they would need to work with the WrapResponseWriter type to access Status(), Body(), and other methods.
WithCustomMessage func(w http.ResponseWriter, r *http.Request) string
Filters []Filter
}
</details>
---
💡 <a href="/samber/slog-http/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
| WithSpanID bool | ||
| WithTraceID bool | ||
| WithClientIP bool | ||
| WithCustomMessage func(w http.ResponseWriter, r *http.Request) string |
There was a problem hiding this comment.
The function signature should use WrapResponseWriter instead of http.ResponseWriter to be consistent with the Filter type (see filters.go:10) and to provide explicit access to additional methods like Status(), Body(), and BytesWritten(). Users would need these methods to generate meaningful custom messages based on the response. Without this, users would need to perform type assertions which is not good API design.
| WithCustomMessage func(w http.ResponseWriter, r *http.Request) string | |
| WithCustomMessage func(w WrapResponseWriter, r *http.Request) string |
| WithSpanID bool | ||
| WithTraceID bool | ||
| WithClientIP bool | ||
| WithCustomMessage func(w http.ResponseWriter, r *http.Request) string |
There was a problem hiding this comment.
The function signature should use WrapResponseWriter instead of http.ResponseWriter to be consistent with the Filter type and to provide explicit access to additional methods like Status(), Body(), and BytesWritten(). This documentation should match the corrected type in the code.
| WithCustomMessage func(w http.ResponseWriter, r *http.Request) string | |
| WithCustomMessage func(w WrapResponseWriter, r *http.Request) string |
| test: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
The addition of fail-fast: false to the test workflow appears unrelated to the PR's stated purpose of adding WithCustomMessage functionality. This change should either be removed from this PR or the PR description should be updated to explain why this change is included. Mixing unrelated changes can make it harder to track the purpose and impact of each change.
No description provided.