Skip to content

Refactor profile collection orchestration and tighten pre-PR checks - #53

Merged
rogeralsing merged 1 commit into
mainfrom
codex/20260320-221504-69bdc6e6
Mar 21, 2026
Merged

rogeralsing merged 1 commit into
mainfrom
codex/20260320-221504-69bdc6e6

Conversation

@rogeralsing

Copy link
Copy Markdown
Contributor

Summary

  • split ProfileCollectionRunner so low-level trace collection, heap snapshot collection, provider building, artifact path building, and shared collection services now live in dedicated types
  • extracted compiler-generated/state-machine/lambda symbol parsing into CompilerGeneratedNameFormatter to slim NameFormatter
  • added regression coverage for trace provider generation and artifact path formatting
  • reran the pre-PR quality gate and confirmed a clean warning-free build plus zero quickdup findings

Testing

  • /tmp/asynkron-profiler-tools/roslynator fix src/ProfileTool/ProfileTool.csproj
  • /tmp/asynkron-profiler-tools/roslynator fix src/ProfilerCore/Asynkron.Profiler.Core.csproj
  • /tmp/asynkron-profiler-tools/roslynator fix tests/Asynkron.Profiler.Tests/Asynkron.Profiler.Tests.csproj
  • dotnet format Asynkron.Profiler.sln
  • dotnet build Asynkron.Profiler.sln -warnaserror
  • dotnet test Asynkron.Profiler.sln -warnaserror
  • quickdup -path src -ext .cs -select 0..20 -min 2 -exclude '.g.,.generated.'

Copilot AI review requested due to automatic review settings March 20, 2026 22:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors ProfileTool’s trace/heap collection orchestration into smaller dedicated components, and extracts compiler-generated symbol formatting into a focused formatter to slim down NameFormatter, with regression tests added for provider/path generation.

Changes:

  • Introduced ProfileCollectionServices, DotnetTraceCollector, HeapSnapshotCollector, DotnetTraceProviderFactory, and ProfileArtifactPathBuilder to split responsibilities out of ProfileCollectionRunner.
  • Extracted compiler-generated/state-machine/lambda parsing into CompilerGeneratedNameFormatter and updated NameFormatter to delegate to it.
  • Added tests covering artifact path formatting and ETW provider string generation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Asynkron.Profiler.Tests/ProfileArtifactPathBuilderTests.cs Adds regression test for timestamped artifact path formatting.
tests/Asynkron.Profiler.Tests/DotnetTraceProviderFactoryTests.cs Adds regression tests for trace provider string generation.
src/ProfileTool/Rendering/NameFormatter.cs Delegates compiler-generated name formatting to extracted formatter.
src/ProfileTool/ProfileCollectionServices.cs New shared services container for collection orchestration (theme, process runner, output, logging).
src/ProfileTool/ProfileCollectionRunner.cs Refactors runner to use new collectors/factories and reduces inline orchestration logic.
src/ProfileTool/ProfileArtifactPathBuilder.cs Centralizes timestamped artifact path building.
src/ProfileTool/HeapSnapshotCollector.cs New heap snapshot collection wrapper around dotnet-gcdump.
src/ProfileTool/DotnetTraceProviderFactory.cs New factory for building dotnet-trace provider strings.
src/ProfileTool/DotnetTraceCollector.cs New trace collector wrapper around dotnet-trace collect.
src/ProfilerCore/Rendering/CompilerGeneratedNameFormatter.cs New formatter for compiler-generated/state-machine/lambda symbol names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

using System;
using System.Collections.Generic;
using System.IO;
using Spectre.Console;

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using Spectre.Console; is unused in this file. With -warnaserror this will fail the build (CS8019). Remove the unused using (or reference a Spectre type if it’s actually needed).

Suggested change
using Spectre.Console;

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,69 @@
using System;
using System.Collections.Generic;

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using System.Collections.Generic; is unused in this file. With -warnaserror this will fail the build (CS8019). Remove the unused using.

Suggested change
using System.Collections.Generic;

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +4
using System;
using System.Collections.Generic;
using Spectre.Console;

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using System.Collections.Generic; is unused in this file. With -warnaserror this will fail the build (CS8019). Remove the unused using.

Copilot uses AI. Check for mistakes.
@rogeralsing
rogeralsing merged commit 90e1437 into main Mar 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants