SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
782 stars 46 forks source link

Add Orleans support #653

Closed sandervanteinde closed 1 month ago

sandervanteinde commented 1 month ago

Adds orleans support to Vogen.

This is my first time contributing to an open source project, so I'm not fully familiar with how it should be done. Any help on what still needs to be done to fully integrate this would be greatly appreciated!

Should fix #385

SteveDunn commented 1 month ago

Adds orleans support to Vogen.

This is my first time contributing to an open source project, so I'm not fully familiar with how it should be done. Any help on what still needs to be done to fully integrate this would be greatly appreciated!

Should fix #385

Thank you very much for the PR! This will make a big difference to everyone that's been patiently waiting for this feature!

You've done a great job navigating around and finding the right places to put things. When Vogen started, the converters needed private access to the value object, e.g. _value etc. With more recent versions of C#, we can use new functionality (namely UnsafeAccessor) that was designed for source generators that lets them access private stuff from outside of the class. An example is the new BSON serialization. It means we can now create these serializers in separate classes that don't need to be inner classes.

I'm happy to jump in and rearrange this. While I'm there, I'll also try to make the structure clearer to make similar things easier to add in the future.

Thanks again for the contribution - let me know if you're happy for me to jump and rearrange to the new style.

sandervanteinde commented 1 month ago

Thanks again for the contribution - let me know if you're happy for me to jump and rearrange to the new style.

Of course! Feel free to put the dots on the i ;-) I'll monitor this PR regardless in case you need Orleans-specific inputs from my end.

I have another suggestion that is unrelated to this PR though, I noticed that Snapshots can sometimes change just because of whitespace changes, while acceptable (there's a different output) I do have a recommendation that I'd like to put forward:

In WriteWorkItems.cs:38 there's a piece of code that parses the SourceText.

If you add these prior to that, it will do some very neat auto-formatting based on Microsoft's own parsing engine:

SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(classAsText);
SyntaxNode root = syntaxTree.GetRoot();
SyntaxNode formatted = root.NormalizeWhitespace();

SourceText sourceText = SourceText.From(formatted.ToFullString(), Encoding.UTF8);

This might reduce the amount of times snapshots changes (and as a bonus, it formats all source generated output ;-) )

SteveDunn commented 1 month ago

Thanks again for the contribution - let me know if you're happy for me to jump and rearrange to the new style.

Of course! Feel free to put the dots on the i ;-) I'll monitor this PR regardless in case you need Orleans-specific inputs from my end.

I have another suggestion that is unrelated to this PR though, I noticed that Snapshots can sometimes change just because of whitespace changes, while acceptable (there's a different output) I do have a recommendation that I'd like to put forward:

In WriteWorkItems.cs:38 there's a piece of code that parses the SourceText.

If you add these prior to that, it will do some very neat auto-formatting based on Microsoft's own parsing engine:

SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(classAsText);
SyntaxNode root = syntaxTree.GetRoot();
SyntaxNode formatted = root.NormalizeWhitespace();

SourceText sourceText = SourceText.From(formatted.ToFullString(), Encoding.UTF8);

This might reduce the amount of times snapshots changes (and as a bonus, it formats all source generated output ;-) )

Great stuff! I'll get to shortly. Thanks for the tip re. formatting! That will certainly save time and effort in trying to align code (to be honest, I gave up trying to align code long ago, as you can probably tell from the mess that is generated! :)

sandervanteinde commented 1 month ago

Thanks again for the contribution - let me know if you're happy for me to jump and rearrange to the new style.

Of course! Feel free to put the dots on the i ;-) I'll monitor this PR regardless in case you need Orleans-specific inputs from my end. I have another suggestion that is unrelated to this PR though, I noticed that Snapshots can sometimes change just because of whitespace changes, while acceptable (there's a different output) I do have a recommendation that I'd like to put forward: In WriteWorkItems.cs:38 there's a piece of code that parses the SourceText. If you add these prior to that, it will do some very neat auto-formatting based on Microsoft's own parsing engine:

SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(classAsText);
SyntaxNode root = syntaxTree.GetRoot();
SyntaxNode formatted = root.NormalizeWhitespace();

SourceText sourceText = SourceText.From(formatted.ToFullString(), Encoding.UTF8);

This might reduce the amount of times snapshots changes (and as a bonus, it formats all source generated output ;-) )

Great stuff! I'll get to shortly. Thanks for the tip re. formatting! That will certainly save time and effort in trying to align code (to be honest, I gave up trying to align code long ago, as you can probably tell from the mess that is generated! :)

It's not like Microsoft made it particularly easy to write source generators... Took me a while to figure out the code I shared above haha

Edit: typos

SteveDunn commented 1 month ago

Hopefully, this should be finished building soon, and be ready for merging. Could you think of a trivial example app to put in the Samples folder? There's a few in there already, like an ASPNET app, and some examples of Onion/Ports & Adapters etc.

sandervanteinde commented 1 month ago

Hopefully, this should be finished building soon, and be ready for merging. Could you think of a trivial example app to put in the Samples folder? There's a few in there already, like an ASPNET app, and some examples of Onion/Ports & Adapters etc.

Added an example, had to add this to the csproj, because the build was failing on these warnings and I couldn't figure out as to why that was.

<NoWarn>$(NoWarn),NU1603,NU1903</NoWarn>

Not sure why the build was failing, maybe you have some insight on that?

SteveDunn commented 1 month ago

Hopefully, this should be finished building soon, and be ready for merging. Could you think of a trivial example app to put in the Samples folder? There's a few in there already, like an ASPNET app, and some examples of Onion/Ports & Adapters etc.

Added an example, had to add this to the csproj, because the build was failing on these warnings and I couldn't figure out as to why that was.

<NoWarn>$(NoWarn),NU1603,NU1903</NoWarn>

Not sure why the build was failing, maybe you have some insight on that?

Thank you! The samples use a local nuget folder and consume the very latest nuget package of Vogen (version 999.99.xxx). I can see that you've used the other .csproj files as an example, so that's great! 👍

SteveDunn commented 1 month ago

This looks great! Thanks again for your contribution