-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
update tools package to the latest version with bug fixes #6937
Conversation
eng/mgmt/Directory.Build.Mgmt.props
Outdated
@@ -1,11 +1,11 @@ | |||
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
<PropertyGroup> | |||
<PkgVersion>0.10.0</PkgVersion> | |||
<PkgVersion>0.12.0-dev.20190715.1</PkgVersion> |
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.
We should be able to eliminate this. I don't think we really need condition on the Import below. msbuild will handle the case where the package already exists.
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.
Ok here why you need this condition, I thought of adding a comment, but I am glad you caught this and now I will add a comment so that it is clear.
When you execute tests for the build tools tasks, it will actually will not restore the package for you during test execution if the project needs to resolve the imports and hence your test will fail.
So in that case, you will have to provide a conditional import and prior to executing tests you will have to restore it manually, hence you need it.
/azp run net - client - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run net - mgmt - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
<PkgLocalDir>$(RepoRoot)\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools\$(PkgVersion)\Sdk</PkgLocalDir> | ||
</PropertyGroup> | ||
|
||
<!-- during execution of tasks tests, this conditional import will satisfy the import resolution. And you will have to manually execute a target to restore the package |
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.
Now that I've got the release pipeline working and publishing for these tools can you please go ahead and update this version to the latest published (0.12.0-dev.20190716.14) and delete the package? If so I'll go ahead and close my PR #6879 instead of us fighting which one merges first :)
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.
@weshaggard I am going to ask @erich-wang to open a new PR to get rid of the checked in package.
This would be a good way for him to update these tools package on his own.
update tools package to the latest version with bug fixes