Conversation
|
CC @jasonmalinowski, @dotnet/roslyn-infrastructure |
jasonmalinowski
left a comment
There was a problem hiding this comment.
Just some questions to make the code more self-documenting.
| <!-- NuGet version --> | ||
| <NuGetReleaseVersion>$(RoslynFileVersionBase)</NuGetReleaseVersion> | ||
| <NuGetPerBuildPreReleaseVersion>$(NuGetPreReleaseVersion)-$(BuildNumberFiveDigitDateStamp)-$(BuildNumberBuildOfTheDayPadded)</NuGetPerBuildPreReleaseVersion> | ||
| <NuGetPackageRoot Condition="'$(NuGetPackageRoot)' == ''">$(NUGET_PACKAGES)</NuGetPackageRoot> <!-- Respect environment variable if set --> |
There was a problem hiding this comment.
Could you add a comment to the file explaining why it's in Versions.props? This seems like it really should be in Settings.props where it was.
There was a problem hiding this comment.
A considerable portion of the values defined in Version.props, and by extension Dependencies.props, are NuGet packages. Not having the root for which to find them reduced the values of these files.
Importing Settings.props is a bit of a heavier hand is it brings along a lot of properties which are specific to building. There are a number of build files that are just tooling that can't always import these values. Hence I broke it up this way.
I've tried to document the intent of these files in the following README.md. Happy to update if you feel necessary
https://github.com/dotnet/roslyn/blob/master/build/Targets/README.md
There was a problem hiding this comment.
Perhaps move nuget related properties independent of versions to a separate NuGet.props like so:
https://github.com/dotnet/interactive-window/blob/master/build/Toolset/NuGet.props
https://github.com/dotnet/symreader-portable/blob/master/build/Toolset/NuGet.props
https://github.com/dotnet/symreader/blob/master/build/Toolset/NuGet.props
https://github.com/dotnet/symstore/blob/master/build/Toolset/NuGet.props
| "Microsoft.CodeAnalysis.Analyzers", | ||
| "Microsoft.VisualBasic", | ||
| "Microsoft.Composition", | ||
| "MicroBuild.*", |
There was a problem hiding this comment.
Where is it documented what this does?
There was a problem hiding this comment.
Latest commit on this PR updated the README.md for RepoUtil to describe this section.
Changes: