InternalLogger - Skip allocating params array when no exception#6142
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdjustments to internal logging and string handling: timestamp formatting in InternalLogger, explicit exception-branching in log message construction, replacement of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
src/NLog/Common/InternalLogger.cs (1)
388-433: Indentation inconsistency in the else branch (lines 425-430).The
string.Concatcall at lines 425-430 uses different indentation compared to its counterpart at lines 405-411. The arguments in the non-timestamp else branch are indented by 4 spaces relative toreturn, while the timestamp branch aligns arguments under the opening parenthesis.💅 Proposed fix for consistent indentation
else { if (ex is null) { return string.Concat( level.ToString(), fieldSeparator, fullMessage); } else { return string.Concat( - level.ToString(), - fieldSeparator, - fullMessage, - " Exception: ", - ex.ToString()); + level.ToString(), + fieldSeparator, + fullMessage, + " Exception: ", + ex.ToString()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NLog/Common/InternalLogger.cs` around lines 388 - 433, The CreateLogLine method has inconsistent indentation in the non-timestamp else branch for the string.Concat call; align the argument indentation to match the timestamp branch style so the arguments are vertically aligned under the opening parenthesis (same as the block that returns TimeSource.Current.Time.ToString(...)). Update the string.Concat block in the else branch (the branch where IncludeTimestamp is false and ex is not null) so its arguments (level.ToString(), fieldSeparator, fullMessage, " Exception: ", ex.ToString()) follow the same indentation pattern as the other string.Concat usages in CreateLogLine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/NLog/Common/InternalLogger.cs`:
- Around line 388-433: The CreateLogLine method has inconsistent indentation in
the non-timestamp else branch for the string.Concat call; align the argument
indentation to match the timestamp branch style so the arguments are vertically
aligned under the opening parenthesis (same as the block that returns
TimeSource.Current.Time.ToString(...)). Update the string.Concat block in the
else branch (the branch where IncludeTimestamp is false and ex is not null) so
its arguments (level.ToString(), fieldSeparator, fullMessage, " Exception: ",
ex.ToString()) follow the same indentation pattern as the other string.Concat
usages in CreateLogLine.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d1781f5c-a895-41ac-acce-065159a38c91
📒 Files selected for processing (4)
src/NLog/Common/InternalLogger.cssrc/NLog/Config/AssemblyExtensionLoader.cssrc/NLog/Config/XmlLoggingConfiguration.cssrc/NLog/Targets/FileArchiveHandlers/BaseFileArchiveHandler.cs
aed7494 to
d77de73
Compare
|
|



No description provided.