Skip to content

Conversation

@kzu
Copy link
Member

@kzu kzu commented Jun 25, 2025

When building the pipeline, sometimes it's useful for Use(...) to conditionally turn themselves off depending on some service or configuration. Rather than inject a passthrough DelegatingHandler needlessly, this allows these cases to return a WhatsAppHandler.Skip instead, which will not be added to the pipeline at all

When building the pipeline, sometimes it's useful for Use(...) to conditionally turn themselves off depending on some service or configuration. Rather than inject a passthrough DelegatingHandler needlessly, this allows these cases to return a WhatsAppHandler.Skip instead, which will not be added to the pipeline at all
@kzu kzu added the enhancement New feature or request label Jun 25, 2025
@kzu kzu requested a review from Copilot June 25, 2025 15:44
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 adds support for conditionally skipping handlers in the WhatsApp pipeline by returning a dedicated Skip handler.

  • Refactors the handler invocations in the pipeline builder to check for Skip.
  • Introduces a Skip handler type in WhatsAppHandler and updates the pipeline test accordingly.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/WhatsApp/WhatsAppHandlerBuilder.cs Updates the pipeline builder to conditionally apply factories.
src/WhatsApp/WhatsAppHandler.cs Adds a new Skip handler and updates related documentation.
src/Tests/PipelineTests.cs Modifies tests to include the new Skip handler logic.
Comments suppressed due to low confidence (1)

src/WhatsApp/WhatsAppHandler.cs:20

  • [nitpick] The class name 'SKipWhatsAppHandler' contains a typo with inconsistent capitalization. Consider renaming it to 'SkipWhatsAppHandler' for clarity and consistency.
    public static IWhatsAppHandler Skip { get; } = new SKipWhatsAppHandler();


// Only keep non-skipping handlers.
if (current != WhatsAppHandler.Skip)
handler = factories[i](handler!, services);
Copy link

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

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

The factory method is invoked twice for the same index. Instead, consider assigning 'handler = current' when the current handler is not Skip to avoid potential side effects from the second invocation.

Suggested change
handler = factories[i](handler!, services);
handler = current;

Copilot uses AI. Check for mistakes.
@kzu
Copy link
Member Author

kzu commented Jun 25, 2025

37 passed 37 passed 8 skipped

🧪 Details on Ubuntu 24.04.2 LTS

from dotnet-retest v0.7.1 on .NET 8.0.17 with 💜 by @devlooped

@kzu kzu merged commit 7fef8c3 into main Jun 25, 2025
6 checks passed
@kzu kzu deleted the dev/SkipHandler branch June 25, 2025 15:59
@devlooped devlooped locked and limited conversation to collaborators Jul 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants