-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make AppendRuntimeIdentifierToPath=true the default #873
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
Conversation
@@ -32,8 +32,6 @@ public void It_builds_a_runnable_output() | |||
var ns = project.Root.Name.Namespace; | |||
var propertyGroup = project.Root.Elements(ns + "PropertyGroup").First(); | |||
propertyGroup.Add(new XElement(ns + "RuntimeIdentifier", runtimeIdentifier)); | |||
|
|||
propertyGroup.Add(new XElement(ns + "AppendRuntimeIdentifierToOutputPath", "true")); |
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 maybe set this to "false" to test out that the RID doesn't get appended to the output path?
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.
Coverage for on and off went in with #847
@MattGertz for approval. |
Just noticed there's no template. Hold on. |
Customer scenario Builds that build with multiple rids can stomp their output on each other because the outputpath is not RID specific. Bugs this fixes: Workarounds, if any Risk Medium - since we are changing output paths. Performance impact Low - we are just choosing a different output path. Is this a regression from a previous update? No. Root cause analysis: Originally we tried making RIDs a publsh-only concept but that didn`t work out. This is a holdover from that time. How was the bug found? Internal testing of other RID changes. |
Yep, I mentioned this one to JoC earlier, let's take it to Shiproom. |
e8d88fc
to
300cdbf
Compare
@dotnet-bot test this please (myget should be back up now) |
300cdbf
to
fe23fcc
Compare
VS Mac and ASP NET folks signed off on this change. Merging. |
We will bring a new insertion PR to shiproom today for approval to get this one. |
Fix #868
@dsplaisted @srivatsn @eerhardt @piotrpMSFT