MarimerLLC / csla

A home for your business logic in any .NET application.
https://cslanet.com
MIT License
1.25k stars 402 forks source link

Features/incremental source generator #4039

Closed luizfbicalho closed 2 months ago

luizfbicalho commented 3 months ago

Closes #3068

@rockfordlhotka I added nullable reference types on the generator, please check if the changes to enable nullable that I added are ok for you

rockfordlhotka commented 3 months ago

@luizfbicalho I've asked @TheCakeMonster for input here. I am not sure this code is current or valid anymore. It was never released into production.

TheCakeMonster commented 3 months ago

@rockfordlhotka You are probably right that it isn't entirely current, but only in the sense that I have not checked it recently. I think it's a useful facility, so it's worth getting it released, unless there is a more modern replacement? I thought I saw an email of a merged PR for being able to serialize POCO types go past in the whizz of emails there have been in the past few days, but I can't keep up with them all at the moment.

The idea of switching to incremental source generation is a good one. It's quite a lot more efficient, I think, so it improves the build and development experience. @JasonBock is, of course, much more knowledgeable in this area than me; I simply did the grunt work, including changing it after his review.

There were a couple of reasons why it didn't get released:

  1. I couldn't remember how to change the nuget build to get it included. This is not something I know anything about, as I've never been in a position to need nuspec. This is the main reason.
  2. I started to wonder if this suffered the same security problem as BinaryFormatter, in that it might be capable of deserializing unintended types. I don't think it suffers quite the same problem, but that's very different from being able to say that I know that it doesn't.
luizfbicalho commented 3 months ago

A worthy set of changes, in my opinion. However, I can't approve them as such, just because I don't have experience of incremental generators. However, I know a man who does @JasonBock ;-)

It's funny that I studied his videos to implement this generator

luizfbicalho commented 3 months ago

Is there anything else I can do about this PR?

rockfordlhotka commented 3 months ago

Not right now. I'll review it when I get a chance.

luizfbicalho commented 2 months ago

I guess what I'm looking for is a readme.md in the Source/Csla.Generators` directory that tells someone how to get started.

I just added it,let me know what do you think of it