-
Notifications
You must be signed in to change notification settings - Fork 789
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
Fix setup issues for Roslyn insertion #1657
Changes from 4 commits
0910ba4
bb43767
3b7644d
c1cb523
72deaba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,15 @@ | |
oldVersion="2.0.0.0-4.4.1.0" | ||
newVersion="4.4.1.0"/> | ||
</dependentAssembly> | ||
<dependentAssembly> | ||
<assemblyIdentity | ||
name="System.ValueTuple" | ||
publicKeyToken="b03f5f7f11d50a3a" | ||
culture="neutral"/> | ||
<bindingRedirect | ||
oldVersion="2.0.0.0-4.4.1.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think value tuple needs this binding redirect. In the future it may need one. but not right now. |
||
newVersion="4.0.0.0"/> | ||
</dependentAssembly> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this related to the bug above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Needed so that NGEN can load the correct dependencies. |
||
</assemblyBinding> | ||
</runtime> | ||
</configuration> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,15 @@ | |
oldVersion="2.0.0.0-4.4.1.0" | ||
newVersion="4.4.1.0"/> | ||
</dependentAssembly> | ||
<dependentAssembly> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Value tuple shouldn't need a binding redirect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @KevinRansom We are referencing version 4.0.0.0, so fsc shouldn't be affected by this change. BUT this is needed for NGEN. Do you have concerns for adding this redirection here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed offline. We can get rid of the dependency. |
||
<assemblyIdentity | ||
name="System.ValueTuple" | ||
publicKeyToken="b03f5f7f11d50a3a" | ||
culture="neutral"/> | ||
<bindingRedirect | ||
oldVersion="2.0.0.0-4.4.1.0" | ||
newVersion="4.0.0.0"/> | ||
</dependentAssembly> | ||
</assemblyBinding> | ||
</runtime> | ||
</configuration> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,4 @@ | |
<Asset Type="Microsoft.VisualStudio.MefComponent" d:Source="Project" d:ProjectName="FSharp.Editor" Path="|FSharp.Editor|" AssemblyName="|FSharp.Editor;AssemblyName|" /> | ||
<Asset Type="Microsoft.VisualStudio.Analyzer" d:Source="Project" d:ProjectName="FSharp.Editor" Path="|FSharp.Editor|" /> | ||
</Assets> | ||
<Prerequisites> | ||
<Prerequisite Id="Microsoft.VisualStudio.Component.CoreEditor" Version="[15.0,16.0)" DisplayName="Visual Studio core editor" /> | ||
</Prerequisites> | ||
</PackageManifest> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we removing these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was needed for a local setup issue by Kevin, but it is failing building in willow for now. Discussed it with @KevinRansom we removed the dependency as it is not required ATM. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These will be required to build with RC builds of VS "15" btw. |
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 don't see this as part of the change?
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.
Is this because you moved it here from vs-integration?
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.
Moved. These are not needed in vsintegration. Only used in setup.