Conversation
I understand that it is in general desirable to not repeat common sections and factor them out into a new file. However, I am a bit lost as to why that means we need now 8 new files contanining configurations of some kind. I think it is a bit extreme and creates more problems than it solves. For example, having to navigate a non-trivial hierarchy of files with complex rules to figure out how a project is built is something I would always want to avoid. I am not an expert in the advanced features of msbuild and I don't really want to have to be one. For example, I would rather repeat the few lines that add xunit to the project in the two test projects, rather than creating a new .props file that adds the XUnit information automatically , but then needs a special condition to test the name of the project to avoid over-applying this rule, plus an extra exception from that rule for a project with a name that ends in "Tests" but is not a test project. Honestly, I find it very difficult now to analyze how the build even works. Are the Directory.build.props and Directory.Build.targets "magically" named to be pulled in by msbuild? or are they explicitly included somewhere? Since a lot of them have the same name I guess their interpretation is somehow dependent on the directory path, are they cumulative? Kind of like .gitignore? If that is indeed how they work, I am not a fan of this type of system (using magically named path-sensitive files) I find it unwieldy. For example, in VS, these files don't show up in the Solution Explorer, so I have to use another way to locate them (I have to look at multiple levels to find them all?) and open them. And once I have these files open in the editor, I can get easily confused as to which file from which directory I am looking at (because they have the same name).
Does the content of this global.json need to be updated from time to time? If yes, how would I figure out what to update it to?
Why? It is vastly more complicated looking. The contents of InternalsVisibleTo.targets are unintelligible to me. I would not be able to debug/maintain it if needed.
Do the snupkg need to be uploaded to NuGet manually (just like we currently do for the nukpg) or is it some other mechanism? |
sebastianburckhardt
left a comment
There was a problem hiding this comment.
Some initial comments.
| <PropertyGroup> | ||
| <TargetFramework>net6.0</TargetFramework> | ||
| <AzureFunctionsVersion>v4</AzureFunctionsVersion> | ||
| <IsTestProject>false</IsTestProject> <!-- Not a unit test project --> |
There was a problem hiding this comment.
Maybe we can simplify the logic and instead set it to "true" when it is a test project. I think that would be less of a maintenance pitfall.
| BeforeTargets="BeforeCompile"> | ||
|
|
||
| <WriteCodeFragment AssemblyAttributes="@(_InternalsVisibleToAttribute)" Language="$(Language)" OutputFile="$(GeneratedInternalsVisibleToFile)"> | ||
| <Output TaskParameter="OutputFile" ItemName="CompileBefore" Condition="'$(Language)' == 'F#'" /> |
There was a problem hiding this comment.
Why is there some check for F# language here?
There was a problem hiding this comment.
This file was borrowed from the dotnet team - so this is a remnant of that. I'll remove the F# check.
| <Project> | ||
|
|
||
| <PropertyGroup> | ||
| <IsTestProject Condition="$(MSBuildProjectName.EndsWith('Tests'))">true</IsTestProject> |
There was a problem hiding this comment.
I don't like this rule since it requires an exception (see comment below). I don't think this needs to be automatic, we can just set IsTestProject explicitly in all the projects that are test projects. I think it would be more readable
This PR is a major refactor of the build targets for this repo.
Directory.Build.propsand similar targets. e.g.: all NuGet packaging props are now common./out/folder. This gives a single folder to delete to clean all build artifacts./out/objfor intermediate,/out/binfor build,/out/pkgfor packages (nuget)snupkg. This is the recommended way to deliver debug symbols, as it is now a separate package that only IDEs download on demand. This keeps nupkg smaller for CI builds.InternalsVisibleTomoved to msbuild item & targetglobal.jsonadded to pin dotnet and msbuild SDKs. This forces consistency between different dev/build environments.objfolders possibly containing duplicate generated code. To solve this, you can run:or: