limbo-works / Limbo.Umbraco.ModelsBuilder

Custom models builder for Umbraco.
MIT License
4 stars 3 forks source link

Build warning "warning CS0108" should be ignored #7

Closed hfloyd closed 2 years ago

hfloyd commented 2 years ago

On Build I noticed a bunch of warnings for properties generated for inherited models.

example: warning CS0108: 'MyModel.SeoTitle' hides inherited member 'Master.SeoTitle'. Use the new keyword if hiding was intended.

This is happening for a "nested" style DocumentType (as opposed to Composition-added properties):

public partial class MyModel: Master, IMaster, ICompLegacyData

I think it is safe to ignore this specific issue.

abjerner commented 2 years ago

Hi @hfloyd

What version are you using?

The latest beta (005) should have a fix to that generated properties gets the new keyboard (see 10da2b2bf845b71f1da748d688c02f01f70cb459). On top of my head, I think that should address the issue - but I can't rule out that there might be some scenarios where you'd still see that warning.

So the issue might already be addressed by 10da2b2bf845b71f1da748d688c02f01f70cb459 - but on the other hand, the #pragma declaration doesn't really hurt, so I'll look into merging your PR 😉 👍

hfloyd commented 2 years ago

Hi @abjerner , I'm using '1.0.0-beta005' but still getting some of those warnings. Specifically it seems to be for Models which inherit from a master doctype (ex: 'Model.SeoTitle' hides inherited member 'Master.SeoTitle'.)

abjerner commented 2 years ago

@hfloyd OK. We have that as well, but I thought I fixed that with the new keyword. Nevertheless, I've now merged #9 so it should be fixed either way.

abjerner commented 2 years ago

I've now pushed v1.0.0-beta006 which contains your PR as well as a fix for #5 😉

hfloyd commented 2 years ago

Thanks for the update! You know, looking closer, I see what the issue is with the CS0108 warning ...

There are OLD ".generated.cs" files which are still getting compiled. For instance, I changed the name of a Doctype from "Model" to "ProductModel" - when I regenerated, the new "ProductModel.generated.cs" file is created - but the OLD "Model.generated.cs" file is still hanging around.

When I open it I see this at the top: image

What do you think - Should all the ".generated.cs" files get deleted before new files are generated?

abjerner commented 2 years ago

I had my suspicions that was the case 😄

We've had some talks about this internally as well, but no conclusion yet.

With OMB, the models were always placed in a single folder. With LMB, models can split out and placed in different folders, which makes it a bit harder to keep track of.

Given the file header added by LMB, I'd imagine I could find all *.generated.cs file in the project generated by LMB, and then delete them before generating the models again.

I've also considered adding the content type GUID to the file somewhere. If so, this could allow the generator to only delete the files it needs to. But it may not work if content types are deleted.

ASP.NET Core also works a bit different from ASP.NET, as updating files could trigger a restart. I haven't experienced problems with this so far, but maybe there could be problems when deleting all generated files first, and then generating the files all over again 🤷‍♂️

It's definitely something I will look a bit more into when I have some more time to work on the package 😉

abjerner commented 2 years ago

Btw - I've created an issue for it here: https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/10

hfloyd commented 2 years ago

I just deleted the older files via Windows Explorer to clean up my project today, and it then builds without any warnings. 😁

I'm not sure if deleting the files while the site is running would mess anything up or not... though I wonder if it would really be different from changing code files while the site is running...? Of course, no emergency on this, whenever you have a chance to reflect on the best course will be fine. Thanks for everything!

abjerner commented 2 years ago

Turns out what I described actually causes more problems that I had thought.

When developing on this package so far, I've used a mix running my test site directly from VS2019 and running it from console via dotnet run. When working this way, I didn't experience any problems. When running from VS2019, the site will restart the LMB saves the updated models to disk, but all files seem to be saved before the server restarts.

On the other hand, when running the site from either VS2022 or via dotnet watch run - possibly because of hot reloading - the site restarts before all model files are saved to disk, so the process doesn't complete.

I've yet to find a solution for this. It seems that it works fine with EMB, but haven't spotted any major differences when running through their code - but I need to look at bit more in to whether they're doing something I've missed.