Make EventSource telemetry module target netstandard1.3 #77
Conversation
|
@karolz-ms, The agreement will cover your contributions to all Microsoft-managed open source projects. |
| @@ -1,11 +1,5 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
| <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'Global.props'))\Global.props" /> | |||
| <Import Project=".\GlobalStaticVersion.props" /> | |||
There was a problem hiding this comment.
this change makes this repository inconsistent with the https://github.com/Microsoft/ApplicationInsights-dotnet Do you think we need to change that one too?
There was a problem hiding this comment.
I think that might be necessary.
The issue is that if the BaseIntermediateOutputPath property is set after Microsoft.Common.props has been imported, then it is too late for it to pick up the .props files generated in that folder by NuGet restore. You can resolve this by always importing Global.props file before Microsoft.Common.props gets imported, but I found it tricky to ensure. An easier way, and this is what I have done here, is to use the special Directory.Build.props file which will automatically get imported beforehand by MSBuild. For more information, see dotnet/msbuild#1603 (comment)
There was a problem hiding this comment.
Is this the same idea? https://github.com/Microsoft/ApplicationInsights-dotnet/pull/466/files Can you please align by names of files
There was a problem hiding this comment.
It is the same problem but alternative solution. I like this one better because it does not require non-default project file structure (i.e. you can say <Project Sdk="Microsoft.NET.Sdk"> and everything still works.
Not sure exactly what you mean "align by names of files"? Do you mean "make it consistent between the two repos"?
There was a problem hiding this comment.
Yes. I didn't dig into the exact solution details. So if you think your solution better - can you please as a minimum create an issue in dotnet repo
There was a problem hiding this comment.
Sounds good, microsoft/ApplicationInsights-dotnet#490 logged
|
Just to clarify: this change will require that the repo is built using a machine pool with MSBuild 15/VS 2017 tooling |
This change also makes the whole AI-logging repo use MSBuild 15 tooling