-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
C#: Refactor common msbuild properties #16521
C#: Refactor common msbuild properties #16521
Conversation
…d.targets in stub generator
86aad27
to
cecaa0d
Compare
@@ -0,0 +1,6 @@ | |||
<Project> |
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.
This file is not strictly needed.
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 this is not strictly needed, are the changes in the stub generator not strictly needed either (I suppose this file currently has the same effect for stubs used by the QL tests as the project local Directory.Build.props
that will be generated going forward)?
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's not strictly needed, because when we run integration tests through sembuild, we're moving the files to the targets folder and then the settings in Directory.Build.props
is not going to be inherited.
At the same time, the stub generator changes are needed, because we're creating a stubs_output
subfolder in the csharp
directory, so there the Directory.Build.props
would be inherited.
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 to me!
Some minor question on parts of the logic.
@@ -0,0 +1,6 @@ | |||
<Project> |
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 this is not strictly needed, are the changes in the stub generator not strictly needed either (I suppose this file currently has the same effect for stubs used by the QL tests as the project local Directory.Build.props
that will be generated going forward)?
This PR is moving the common MsBuild properties to a top level
Directory.Build.props
file.