Skip to content

Conversation

@JamieMagee
Copy link
Member

One of the new features in Spectre.Console.Cli was support for top-level CancellationToken support. This means, that we will be able to pipe cancellation support the entire way through Component Detection. This is a first trial run, passing the CancellationToken through to ScanExecutionService and DetectorProcessingService.

Additionally, I added XML documentation and migrated to primary constructors.

@JamieMagee JamieMagee requested a review from a team as a code owner November 14, 2025 05:22
Copilot finished reviewing on behalf of JamieMagee November 14, 2025 05:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces CancellationToken support throughout the Component Detection command pipeline, leveraging new capabilities from Spectre.Console.Cli. The changes also include migration to primary constructors and addition of XML documentation for better code clarity.

Key changes:

  • Added CancellationToken parameters to IScanExecutionService.ExecuteScanAsync and IDetectorProcessingService.ProcessDetectorsAsync interfaces
  • Updated ScanCommand to pass cancellation tokens through the execution chain
  • Migrated ListDetectorsCommand and ListDetectorsSettings to use primary constructors

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
IScanExecutionService.cs Added XML documentation and CancellationToken parameter to ExecuteScanAsync method
ScanExecutionService.cs Added CancellationToken parameter to ExecuteScanAsync implementation and XML documentation
IDetectorProcessingService.cs Added XML documentation and CancellationToken parameter to ProcessDetectorsAsync method
ScanCommand.cs Updated to propagate CancellationToken to ExecuteScanAsync calls and added parameter documentation
ListDetectorsCommand.cs Migrated from traditional constructor to primary constructor pattern
ListDetectorsSettings.cs Migrated from class with empty body to primary constructor syntax

@JamieMagee JamieMagee force-pushed the users/jamagee/cancellation-token branch from c582378 to edadde4 Compare November 14, 2025 05:29
@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.3%. Comparing base (8216adf) to head (9a382ef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...on.Orchestrator.Tests/Commands/ScanCommandTests.cs 0.0% 0 Missing and 5 partials ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1530   +/-   ##
=====================================
  Coverage   90.3%   90.3%           
=====================================
  Files        418     418           
  Lines      35300   35299    -1     
  Branches    2188    2188           
=====================================
  Hits       31877   31877           
  Misses      2980    2980           
+ Partials     443     442    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot finished reviewing on behalf of JamieMagee November 14, 2025 05:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

@JamieMagee JamieMagee merged commit 58a3de5 into main Nov 14, 2025
30 of 31 checks passed
@JamieMagee JamieMagee deleted the users/jamagee/cancellation-token branch November 14, 2025 17:44
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.

3 participants