CZEMacLeod
commented
2 years ago Hi @leusbj.
I'm taking some time to review this as it alters quite a few things about the way the project it put together. The splitting of the Directory.Build.props/targets files looks good.
I am a little nervous of the changes to the DefaultItems sections. There are existing issues which were closed which don't look like they are preserved in the new methodology. Also, the adding items then removing them is a known performance issue - especially with large projects (and web applications can have large numbers of static files). This is why the original system used then DefaultItemExcludes extensively.
Another issue seems to be that the web.*.config files are no longer handled the same. At the moment the transforms based on the Configurations (Debug, Release, etc.) are included as none for compile time transform. Other versions are assumed to be content so they can be used in deployment scenarios (such as msdeploy with azure pipelines). I don't think that is taken into account now either.
I do quite like the way you've added the Razor files in, as it will probably make using RazorGenerator.MsBuild easier - although I do tend to use that in libraries not the 'head' Web Application Project; others may well use it in a single project scenario.
I also appreciate that there is a lot more documentation / comments in your versions of these files than the originals (perhaps why you aren't aware of the reasons behind some of the original 'code').
Would you be willing to split this into smaller parts so we can review them individually, and get things brought in more easily, rather than a single, more monolithic 're-write'?
leusbj
The Directory.Build.props/targets was split to differently effect the /src and /samples directories. This allows setting EnableDefaultNoneItems == true for just the nuget package projects under the /src directory.
Also the DefaultItems.props/targets was adjusted to mimic the .netcore web application style of globbing.