Repository navigation
fix: eliminate CS0618 and IL3000 build warnings - #389
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 4b73735e882c123779afff49aa93cef4ff1762e1 and 1d331f7. 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ 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)
📝 WalkthroughWalkthroughThis PR updates NetworkSharedState to apply Segoe UI via SKTypeface.FromFamilyName on three chart axis paints, changes AboutViewModel.BuildStamp() to derive the build date from AppContext.BaseDirectory (checking SysManager.exe then SysManager.dll), and adds a 0.48.20 entry to CHANGELOG documenting both fixes. ChangesDeprecation Warning Elimination (0.48.20)
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Use AppContext.BaseDirectory instead of Assembly.Location which | ||
| // returns empty string in single-file publish (IL3000). | ||
| var dir = AppContext.BaseDirectory; | ||
| var exe = Path.Combine(dir, "SysManager.exe"); |
| if (File.Exists(exe)) | ||
| return File.GetLastWriteTime(exe).ToString("dd MMM yyyy"); | ||
| // Fallback: try the DLL | ||
| var dll = Path.Combine(dir, "SysManager.dll"); |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SysManager/SysManager/ViewModels/NetworkSharedState.cs (1)
448-479:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDispose SKTypeface instances created in axis factory methods.
The axis factory methods create new
SKTypefaceinstances viaFromFamilyName("Segoe UI")(lines 453, 462, 463, 477), but these are never disposed.SKTypefaceis an unmanaged SkiaSharp resource that must be released to prevent memory leaks.The
Dispose()method already handles legend/tooltip paint typefaces (lines 492-495), but the axis paint typefaces are not tracked or disposed.♻️ Proposed fix to track and dispose axis paint typefaces
Store the axis paints as fields and dispose their typefaces in
Dispose():public Axis[] LatencyXAxes { get; } public Axis[] LatencyYAxes { get; } public ObservableCollection<ISeries> TraceSeries { get; } = new(); public Axis[] TraceXAxes { get; } public Axis[] TraceYAxes { get; } + + private readonly List<SolidColorPaint> _axisPaints = new(); public SolidColorPaint LegendTextPaint { get; } = new(SKColor.Parse("E6E9EE")) { SKTypeface = SKTypeface.FromFamilyName("Segoe UI") };Update the factory methods to track the paints:
internal static Axis BuildTimeAxis() => new() { Labeler = v => new DateTime((long)v).ToString("HH:mm:ss"), TextSize = 12, NamePaint = new SolidColorPaint(SKColor.Parse("A3ADBF")), - LabelsPaint = new SolidColorPaint(SKColor.Parse("E6E9EE")) { SKTypeface = SKTypeface.FromFamilyName("Segoe UI") }, + LabelsPaint = TrackPaint(new SolidColorPaint(SKColor.Parse("E6E9EE")) { SKTypeface = SKTypeface.FromFamilyName("Segoe UI") }), SeparatorsPaint = new SolidColorPaint(SKColor.Parse("2A3244").WithAlpha(80)) };Add a helper method (make the factory methods non-static or pass the list):
+ private SolidColorPaint TrackPaint(SolidColorPaint paint) + { + _axisPaints.Add(paint); + return paint; + }Dispose the typefaces in
Dispose():// Dispose SKTypeface (unmanaged SkiaSharp memory) — LEAK-003 LegendTextPaint.SKTypeface?.Dispose(); LegendBackgroundPaint.SKTypeface?.Dispose(); TooltipTextPaint.SKTypeface?.Dispose(); TooltipBackgroundPaint.SKTypeface?.Dispose(); + foreach (var paint in _axisPaints) + paint.SKTypeface?.Dispose(); }Alternative simpler fix: Create a single shared
SKTypefaceinstance and reuse it for all axis paints instead of creating four separate instances.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager/ViewModels/NetworkSharedState.cs` around lines 448 - 479, The axis factory methods BuildTimeAxis, BuildValueAxis, and BuildHopAxis each call SKTypeface.FromFamilyName("Segoe UI") but never release the SKTypeface; update the code to either (a) create a single shared SKTypeface field (e.g. _axisTypeface) that all axis paints reuse, assign that SKTypeface to NamePaint/LabelsPaint in BuildTimeAxis/BuildValueAxis/BuildHopAxis, and dispose _axisTypeface in Dispose(), or (b) track the created paints as instance fields (e.g. _timeAxisLabelsPaint, _valueAxisNamePaint, _hopAxisLabelsPaint) and in Dispose() call Dispose() on their SKTypefaces; ensure the Dispose() method is updated to release whichever approach you choose and remove any per-call FromFamilyName calls that would leak.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@SysManager/SysManager/ViewModels/NetworkSharedState.cs`:
- Around line 448-479: The axis factory methods BuildTimeAxis, BuildValueAxis,
and BuildHopAxis each call SKTypeface.FromFamilyName("Segoe UI") but never
release the SKTypeface; update the code to either (a) create a single shared
SKTypeface field (e.g. _axisTypeface) that all axis paints reuse, assign that
SKTypeface to NamePaint/LabelsPaint in
BuildTimeAxis/BuildValueAxis/BuildHopAxis, and dispose _axisTypeface in
Dispose(), or (b) track the created paints as instance fields (e.g.
_timeAxisLabelsPaint, _valueAxisNamePaint, _hopAxisLabelsPaint) and in Dispose()
call Dispose() on their SKTypefaces; ensure the Dispose() method is updated to
release whichever approach you choose and remove any per-call FromFamilyName
calls that would leak.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac3f882e-f844-4412-a7c7-69bc66f87d85
📥 Commits
Reviewing files that changed from the base of the PR and between 7580420 and 4b73735e882c123779afff49aa93cef4ff1762e1.
📒 Files selected for processing (3)
CHANGELOG.mdSysManager/SysManager/ViewModels/AboutViewModel.csSysManager/SysManager/ViewModels/NetworkSharedState.cs
📜 Review details
⏰ 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). (1)
- GitHub Check: UI automation tests
🧰 Additional context used
🪛 GitHub Check: CodeQL
SysManager/SysManager/ViewModels/AboutViewModel.cs
[notice] 515-515: Call to 'System.IO.Path.Combine' may silently drop its earlier arguments
Call to 'System.IO.Path.Combine' may silently drop its earlier arguments.
[notice] 519-519: Call to 'System.IO.Path.Combine' may silently drop its earlier arguments
Call to 'System.IO.Path.Combine' may silently drop its earlier arguments.
🔇 Additional comments (2)
CHANGELOG.md (1)
9-17: LGTM!SysManager/SysManager/ViewModels/AboutViewModel.cs (1)
508-526: 💤 Low valueCodeQL Path.Combine warnings are false positives.
Lines 515 and 519 are flagged by static analysis for potential silent argument dropping in
Path.Combine. This is a false positive because"SysManager.exe"and"SysManager.dll"are hardcoded relative filenames. Per .NET documentation,Path.Combineonly discards previous path components if a subsequent argument is a rooted path (starting with\or a drive letter likeC:). Since these filenames neither start with a separator nor a drive letter, they remain relative paths, and the combination withAppContext.BaseDirectoryis safe.The implementation correctly handles single-file publish scenarios where
Assembly.Locationreturns an empty string.
- NetworkSharedState: replace obsolete SkiaPaint.FontFamily with SKTypeface.FromFamilyName() on 4 axis label/name paints - AboutViewModel: replace Assembly.Location (empty in single-file publish) with AppContext.BaseDirectory + exe/dll lookup for build stamp
4b73735 to
1d331f7
Compare
- NetworkSharedState: replace obsolete SkiaPaint.FontFamily with SKTypeface.FromFamilyName() on 4 axis label/name paints - AboutViewModel: replace Assembly.Location (empty in single-file publish) with AppContext.BaseDirectory + exe/dll lookup for build stamp Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
3 participants
Summary
Eliminates persistent CS0618 and IL3000 build warnings.
CS0618 — SkiaPaint.FontFamily obsolete (4 locations)
IL3000 — Assembly.Location empty in single-file publish
Result
Build