Skip to content
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

Use a source generator to generate the dynamic ids for message types (DB-69) #3684

Merged

Conversation

timothycoleman
Copy link
Contributor

@timothycoleman timothycoleman commented Dec 15, 2022

Added: Source generator for dynamic message type ids

This saves on boilerplate code when adding new messages, but is also anticipated to be of use with the enhanced telemetry work


namespace EventStore.SourceGenerators.Messaging {

public class RootMessageAttribute : Attribute {
Copy link

Choose a reason for hiding this comment

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

From a consumption perspective it's better to have the attributes defined in the "main"-code, not generated.

E.g. when you start a new project, then there's no attribute before the generator creates it. That can cause troubles with VS (even today).
Thus the common pattern (at least in dotnet-org) seems to be to have the needed attributes be parts of the shipped binaries, so the generator just "consumes" them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! ive adjusted this

// 3. Process the resulting tree top-down, adding in the extra generated code that we want.
namespace EventStore.SourceGenerators.Messaging {
[Generator]
public class StatsMessageSourceGenerator : ISourceGenerator {
Copy link

Choose a reason for hiding this comment

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

The generator should use incremental version.

The ISourceGenerator runs -- put simply -- on every keystroke, thus can slow down dev-experience on medium / large projects significantly. The incremental generator is more advanced, so that only on actual relevant change it's active. IMO also the programming model is more natural and aligned to how compilers work (parse and emit is more separated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks i'll take a look

@timothycoleman timothycoleman force-pushed the timothycoleman/source-generate-message-type-ids branch from bb353c6 to 7da100b Compare December 19, 2022 09:43
@timothycoleman timothycoleman marked this pull request as ready for review December 19, 2022 09:43
@timothycoleman timothycoleman changed the title Use source generator to generate ids for message types Use a source generator to generate the dynamic ids for message types Dec 19, 2022
This saves on boilerplate code when adding new messages, but is also anticipated to be of use with the enhanced telemetry work
@timothycoleman timothycoleman force-pushed the timothycoleman/source-generate-message-type-ids branch from 7da100b to 3e3cd05 Compare December 19, 2022 10:07
@pvanbuijtene pvanbuijtene merged commit 7765ec6 into master Dec 20, 2022
@pvanbuijtene pvanbuijtene deleted the timothycoleman/source-generate-message-type-ids branch December 20, 2022 15:54
@timothycoleman timothycoleman changed the title Use a source generator to generate the dynamic ids for message types Use a source generator to generate the dynamic ids for message types (DB-69) May 3, 2023
@linear
Copy link

linear bot commented May 3, 2023

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.

None yet

4 participants