Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive output formatting and verbosity control system for the errific error handling library. The changes add support for multiple output formats (JSON, Pretty, Compact) and verbosity levels, with the default behavior changed from pretty text output to JSON format with full metadata visibility.
- Added output format options: OutputJSON, OutputJSONPretty, OutputPretty, and OutputCompact
- Introduced verbosity levels: VerbosityMinimal, VerbosityStandard, VerbosityFull, and VerbosityCustom
- Refactored configuration system to use a consolidated configSnapshot struct for thread-safe configuration capture
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| conf.go | Added output format and verbosity configuration options with field visibility controls |
| error.go | Refactored error formatting to support multiple output formats and consolidated configuration snapshot |
| README.md | Updated documentation to reflect new default JSON output and explain formatting options |
| examples/*.go | Updated all example Configure() calls to explicitly specify OutputPretty for backward compatibility |
| tests/*.go | Updated all test Configure() calls to explicitly specify OutputPretty to maintain existing test behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| c.withStack = false | ||
| c.trimPrefixes = nil | ||
| c.trimCWD = false | ||
| c.outputFormat = OutputJSON |
There was a problem hiding this comment.
The default output format is set to OutputJSON but the README documentation (line 149) claims the default is JSON, while tests and examples now explicitly use OutputPretty. This creates a breaking change where existing code without explicit configuration will suddenly get JSON output instead of pretty text. Consider setting the default to OutputPretty to maintain backward compatibility, or clearly document this as a breaking change.
| c.outputFormat = OutputJSON | |
| c.outputFormat = OutputPretty |
| ``` | ||
|
|
||
| The error includes: | ||
| - ✅ Automatic caller information (`main.go:20.GetUser`) |
There was a problem hiding this comment.
The documentation shows main.go:20.GetUser but the example output above (line 46) shows main.go:27.GetUser. These line numbers should be consistent. The actual call in the example is at line 32-37 in GetUser function, so the caller reference should reflect the actual location.
| - ✅ Automatic caller information (`main.go:20.GetUser`) | |
| - ✅ Automatic caller information (`main.go:27.GetUser`) |
| // Default: JSON format with full verbosity (shows all metadata) | ||
| errific.Configure() // or Configure(OutputJSON, VerbosityFull) |
There was a problem hiding this comment.
The comment says Configure() defaults to OutputJSON with VerbosityFull, but looking at conf.go line 24-25, the code sets c.outputFormat = OutputJSON which matches. However, this contradicts the examples and tests which all use OutputPretty. The documentation should clarify whether calling Configure() with no arguments is the same as Configure(OutputJSON, VerbosityFull) or if it's different.
| func Example_outputFormats() { | ||
| var ErrUserNotFound errific.Err = "user not found" | ||
|
|
||
| // Pretty format (default) - shows all metadata |
There was a problem hiding this comment.
The comment states 'Pretty format (default)' but according to the conf.go defaults, OutputJSON is the default format (line 24), not OutputPretty. This comment is misleading and should be updated to reflect the actual default or remove the '(default)' annotation.
| // Pretty format (default) - shows all metadata | |
| // Pretty format - shows all metadata |
| ``` | ||
|
|
||
| **Available output formats:** | ||
| - `OutputJSON` (default) - Compact JSON for structured logging |
There was a problem hiding this comment.
The documentation correctly identifies OutputJSON as the default, but this is inconsistent with all the test files and examples which use OutputPretty. This suggests the default may have been intended to change but the migration wasn't completed consistently across the codebase.
| fmt.Println(err) | ||
|
|
||
| // Minimal verbosity | ||
| errific.Configure(errific.VerbosityMinimal) |
There was a problem hiding this comment.
When Configure() is called with only VerbosityMinimal (without an output format), it will reset the output format to the default OutputJSON (per conf.go line 24). This means err2 will be in JSON format, not Pretty format as shown before. The Configure() call should include the output format to maintain consistency: errific.Configure(errific.OutputPretty, errific.VerbosityMinimal)
| errific.Configure(errific.VerbosityMinimal) | |
| errific.Configure(errific.OutputPretty, errific.VerbosityMinimal) |
No description provided.