Conversation
WalkthroughTwo boolean properties on CallSiteLayoutRenderer were marked obsolete and hidden from editor browsing; all usages are wrapped with pragma directives to suppress obsolete warnings. Unit tests were updated to remove explicit uses and test cases for these options. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
snakefoot
force-pushed
the
cleanup_obsolete
branch
from
9e69dfc to
030519f
Compare
…eanNamesOfAsyncContinuation
snakefoot
force-pushed
the
cleanup_obsolete
branch
from
030519f to
53cbb75
Compare
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/NLog.UnitTests/LayoutRenderers/CallSiteTests.cs (1)
1197-1197: Same naming nit as aboveSee comment on Line 1167 about dropping “IsTrue” from the test method name for clarity.
🧹 Nitpick comments (1)
tests/NLog.UnitTests/LayoutRenderers/CallSiteTests.cs (1)
1167-1167: Consider renaming test to remove “IsTrue” from its nameWith the option hidden/obsolete and defaults doing the cleaning, the method name
LogAfterTaskRunAwait_CleanNamesOfAsyncContinuationsIsTrue_ShouldCleanMethodNamecan be shortened (e.g.,LogAfterTaskRunAwait_ShouldCleanMethodName) to avoid implying a tunable setting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/NLog/LayoutRenderers/CallSiteLayoutRenderer.cs(6 hunks)tests/NLog.UnitTests/LayoutRenderers/CallSiteTests.cs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/NLog/LayoutRenderers/CallSiteLayoutRenderer.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (5)
tests/NLog.UnitTests/LayoutRenderers/CallSiteTests.cs (5)
415-415: LGTM: removed obsolete cleaning flag; class-only callsite remains correctThe updated layout
${callsite:ClassName=true:MethodName=false}matches the test assertion intent and avoids obsolete configuration.
448-448: LGTM: removed obsolete cleaning flag with IncludeNamespace override
${callsite:ClassName=true:MethodName=false:IncludeNamespace=false}preserves the previous semantics while no longer referencing the obsolete option.
1167-1167: LGTM: rely on default async-continuation cleaningDropping
cleannamesofasynccontinuationsand using${callsite:classname=false}aligns with the obsoletion and keeps the assertion stable.
1197-1197: LGTM: remove obsolete async-continuation flag; class name + namespace case remains covered
${callsite:classname=true:includenamespace=true}is the right replacement and matches the expectation.
382-382: ✅ No remaining obsolete CallSite flags detected – Ready to merge.A project-wide search confirms that all references to the now-obsolete
CleanNamesOfAnonymousDelegatesandCleanNamesOfAsyncContinuationsflags exist only as:
- Obsolete properties in
src/NLog/LayoutRenderers/CallSiteLayoutRenderer.cs(marked with[Obsolete]and left with their defaulttruevalues for backward compatibility).- Changelog entries in
CHANGELOG.mddocumenting their deprecation.- Unit tests under
tests/NLog.UnitTests/LayoutRenderers/CallSiteTests.csthat verify the default cleaning behavior (no tests set these flags tofalse).No occurrences of these flags were found in XML/layout configurations, tests setting them to
false, or any other code paths. All cleanup has been applied correctly.
|
1 participant
Unit-tests on Linux frequently fails when CleanNamesOfAnonymousDelegates=false or CleanNamesOfAsyncContinuation=false when running on NET8 (instead of NET6)
Maybe some new optimization in the NET8 JIT-compiler (inlines so cleanup is not needed)
See also #5923