Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should we add this to all our tests projects?
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 honestly don't know. I was just doing the minimal change to work around the VSmac bug. I'm not sure why it's not complaining about the other projects.
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.
https://github.com/dotnet/machinelearning/blob/master/test/Directory.Build.props
we have it specified for whole directory. And it works on Windows, I wonder is it VS mac specific
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.
Let me add @eerhardt as most experienced person with builds and projects
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.
Yes, the bug is 100% VSmac-specific. I hadn't noticed the props file but you're definitely right.
Up to you folks if you want to carry a work-around or just tell users to ignore the error until VSmac gets fixed.
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.
@mhutch @mrward - any idea how long until the VSmac bug is fixed? If it should be fixed soon (like in the next release) maybe we shouldn't take this. But if it is going to be a while, we probably should make it so VSmac works with our repo.
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.
If we go ahead with this change, I agree that all the test projects should be fixed. We should probably just take the TargetFramework setting out of the test/Directory.Build.props file, and duplicate them in all the test .csprojs.
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.
It is currently targeted for VS Mac 7.6, which would be the next release, but there is no fix as yet.
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.
@eerhardt I've updated the PR to move
TargetFrameworkout of the props file and into each test csproj.