-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace project-level Common.props imports with Directory.Build.props #852
replace project-level Common.props imports with Directory.Build.props #852
Conversation
I'm still working on getting |
src/Directory.Build.props
Outdated
@@ -0,0 +1,5 @@ | |||
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
|
|||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'Common.props'))\Common.props" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're opted into v15-only features already, you could simplify this file by dropping ToolsVersion
and the namespace and changing this line to
<Import Project="$([MSBuild]::GetPathOfFileAbove('Common.props'))" />
(That property function is new to MSBuild 15: dotnet/msbuild#1277)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except it looks like there's one inadvertant change to a TestAssets project.
@@ -1,6 +1,6 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intentionall? I don't think the test projects would need to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll back out this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonsequitur This still isn't backed out
8d102ab
to
e657a93
Compare
@srivatsn This is an infrastructure-only change. |
You can merge without needing approval since this is a infra change. |
e657a93
to
ed34cb6
Compare
[release/3.0.1xx] Switch to prod pool
This removes the Common.props imports from the projects under /test and /src and replaces them with Directory.Build.props.
This addresses #802.