-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[BREAKING] Obsoleting ReflectingExecutor in favor of source gen #3380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ce Generators and updating some samples to use new pattern. This commit deprecates the reflection-based handler discovery approach in favor of the new [MessageHandler] attribute with source generation. Changes: - Add [Obsolete] to ReflectingExecutor<T>, IMessageHandler<T>, IMessageHandler<T,R> - Add #pragma to suppress warnings in internal reflection code - Update Concurrent sample to use new [MessageHandler] pattern - Add Directory.Build.props for samples to include generator - Add documentation files explaining the migration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
I will hold off on merging this until the docs are updated to clearly explain the new pattern. |
There was a problem hiding this 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 obsoletes the reflection-based ReflectingExecutor<TExecutor> pattern in favor of a source generation approach using the [MessageHandler] attribute. The changes ensure backward compatibility while guiding developers to the new recommended pattern.
Changes:
- Marked
ReflectingExecutor<TExecutor>andIMessageHandler<T>interfaces as obsolete with clear migration guidance - Added pragma warnings to suppress obsolete warnings in internal code and tests that need backward compatibility
- Updated one sample executor to demonstrate the new
[MessageHandler]pattern - Added source generator project reference to the Workflows samples Directory.Build.props
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ReflectingExecutor.cs |
Added [Obsolete] attribute with migration guidance to use [MessageHandler] attribute |
IMessageHandler.cs |
Marked both IMessageHandler<TMessage> and IMessageHandler<TMessage, TResult> interfaces as obsolete |
Executor.cs |
Added pragma to suppress obsolete warnings for internal backward compatibility |
StatefulExecutor.cs |
Added pragma to suppress obsolete warnings for internal backward compatibility |
RouteBuilderExtensions.cs |
Added pragma to suppress obsolete warnings for internal backward compatibility |
MessageHandlerInfo.cs |
Added pragma to suppress obsolete warnings for internal backward compatibility |
01_Simple_Workflow_Sequential.cs |
Added pragma to suppress obsolete warnings when testing legacy reflection-based pattern |
02_Simple_Workflow_Condition.cs |
Added pragma to suppress obsolete warnings when testing legacy reflection-based pattern |
03_Simple_Workflow_Loop.cs |
Added pragma to suppress obsolete warnings when testing legacy reflection-based pattern |
ReflectionSmokeTest.cs |
Added pragma to suppress obsolete warnings when testing legacy reflection-based pattern |
Directory.Build.props |
Added source generator project reference for samples using [MessageHandler] attribute |
Concurrent/Program.cs |
Migrated ConcurrentStartExecutor from Executor<string> to base Executor with [MessageHandler] attribute |
|
There has been some pushback on this since Source Generators do not work with F# |
FWIW, even without the source generator I still think the right answer is deleting the reflection goo and just registering handlers directly. F# still has that experience. |
Motivation and Context
Description
Contribution Checklist