-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
v3: Updates to fiberlog benchmarks and documentation #3059
Conversation
WalkthroughThe recent updates focus on improving the Fiber library's logging and testing functionalities. Changes to the logging documentation clarify concepts, usage, and configuration. In the benchmark tests, the addition of parallel execution support showcases an effort to enhance the evaluation of logging performance. Updated function names in various benchmark tests ensure better consistency and readability. Changes
Poem
Tip AI model upgrade
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3059 +/- ##
==========================================
- Coverage 83.04% 83.01% -0.03%
==========================================
Files 115 115
Lines 8315 8315
==========================================
- Hits 6905 6903 -2
- Misses 1077 1078 +1
- Partials 333 334 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
Outside diff range and nitpick comments (3)
docs/api/log.md (3)
Line range hint
83-93
: Fix markdown issues.The changes improve clarity, but there are markdown issues.
- For projects that require a simple, global logging function to print messages at any time, Fiber provides a global log. + For projects that require a simple, global logging function to print messages at any time, Fiber provides a global log. ### Global log
Line range hint
116-127
: Fix markdown issues.The changes improve clarity, but there are markdown issues.
- log.SetLevel sets the minimum level of logs that will be output. The default log level is LevelTrace. + `log.SetLevel` sets the minimum level of logs that will be output. The default log level is `LevelTrace`. ### Set Level
Line range hint
130-142
: Fix markdown issues.The changes improve clarity, but there are markdown issues.
- log.SetOutput sets the output destination of the logger. By default, the logger outputs logs to the console. + `log.SetOutput` sets the output destination of the logger. By default, the logger outputs logs to the console. ### Set output
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/api/log.md (10 hunks)
- log/default_test.go (1 hunks)
- log/fiberlog_test.go (1 hunks)
- middleware/cors/cors_test.go (6 hunks)
Files skipped from review due to trivial changes (2)
- log/default_test.go
- middleware/cors/cors_test.go
Additional context used
LanguageTool
docs/api/log.md
[grammar] ~47-~47: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... Please use it with caution. ### Basic Logging Logs of different levels can be directly pri...(REPEATED_VERBS)
[grammar] ~60-~60: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...he system is down.") ``` ### Formatted Logging Logs of different levels can be formatted be...(REPEATED_VERBS)
Markdownlint
docs/api/log.md
44-44: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
47-47: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
60-60: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
71-71: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
82-82: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
115-115: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
Additional comments not posted (10)
log/fiberlog_test.go (6)
73-79
: LGTM!The
Benchmark_DefaultSystemLogger
function is correctly implemented.
81-92
: LGTM!The
Benchmark_SetLogger
function is correctly implemented.
94-140
: LGTM!The
Benchmark_Fiberlog_SetLevel
function is correctly implemented and covers multiple test cases.
142-150
: LGTM!The
Benchmark_DefaultSystemLogger_Parallel
function is correctly implemented.
152-165
: LGTM!The
Benchmark_SetLogger_Parallel
function is correctly implemented.
167-215
: LGTM!The
Benchmark_Fiberlog_SetLevel_Parallel
function is correctly implemented and covers multiple test cases.docs/api/log.md (4)
8-10
: LGTM!The introductory changes improve clarity and completeness.
169-176
: LGTM!The changes improve clarity and completeness.
156-166
: Fix markdown issues.The changes improve clarity, but there are markdown issues.
- The following example will write the logs to both test.log and stdout: + The following example will write the logs to both `test.log` and `stdout`: ### Writing logs to both console and fileLikely invalid or redundant comment.
71-72
: Fix markdown issues.The changes improve clarity, but there are markdown issues.
- Print a message with key-value pairs. If the key and value are not paired correctly, the log will output KEYVALS UNPAIRED. + Print a message with key-value pairs. If the key and value are not paired correctly, the log will output `KEYVALS UNPAIRED`. ### Key-Value LoggingLikely invalid or redundant comment.
Tools
Markdownlint
71-71: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
Description
cors
benchmarks to have consistent nameslog.md
.Related to #3048
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change