Skip to content

downgrade to tooling 1.0 - #2380

Merged
matthid merged 6 commits into
fsprojects:masterfrom
0x53A:downgrade-netstandard
Jun 1, 2017
Merged

matthid merged 6 commits into
fsprojects:masterfrom
0x53A:downgrade-netstandard

Conversation

@0x53A

@0x53A 0x53A commented May 29, 2017

Copy link
Copy Markdown
Contributor

@0x53A

0x53A commented May 29, 2017

Copy link
Copy Markdown
Contributor Author

I didn't run any tests locally, let's see what the CI says.

@0x53A

0x53A commented May 30, 2017

Copy link
Copy Markdown
Contributor Author

@forki What's up with the build? :D

@matthid

matthid commented May 30, 2017

Copy link
Copy Markdown
Member

Can you pick the MSBuild fixes from #2362?
Sorry #2362 is not ready to merge jet, and I need a couple of days to fix ... I will take care of it if you can't

@0x53A

0x53A commented May 30, 2017

Copy link
Copy Markdown
Contributor Author

I would prefer to just add the nuget FSharp.Compiler.Tools instead of hardcoding a few different paths. What do you think?

@matthid

matthid commented May 30, 2017

Copy link
Copy Markdown
Member

Yes if it works I'm fine with that. nice!

@0x53A

0x53A commented May 30, 2017

Copy link
Copy Markdown
Contributor Author

Build now works, but one unit test failed. On it!

The great thing about the nuget is that this will allow compatibility with VS 2015 and F# vLatest. For example, if you use struct tuples, you will get red squiggles in the ide, but can still build and run / debug.

@matthid

matthid commented May 30, 2017

Copy link
Copy Markdown
Member

Yes very nice! btw you can test with the appveyor artefact how this change will impact your scenario (sorry i'm on mobile today and tomorrow...) or just build locally obviously

@0x53A

0x53A commented May 30, 2017

Copy link
Copy Markdown
Contributor Author

wp_ss_20170531_0001
Av is green, travis failed with a weird error.

Please review if the unit test changes are actually correct.

@matthid matthid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep everything looks fine, just some minor stuff. You can ignore the travis failure. I'm on that in my other PR

| DotNetFramework FrameworkVersion.V5_0 -> [ DotNetFramework FrameworkVersion.V4_6_2; DotNetStandard DotNetStandardVersion.V1_5 ]
| DotNetFramework FrameworkVersion.V4_6_3 -> [ DotNetFramework FrameworkVersion.V4_6_2 ]
| DotNetFramework FrameworkVersion.V4_7 -> [ DotNetFramework FrameworkVersion.V4_6_3]
| DotNetFramework FrameworkVersion.V5_0 -> [ DotNetFramework FrameworkVersion.V4_6_2 ]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'5.0' -> '4.7'

refs |> shouldNotContain @"..\System.Security.Cryptography.Algorithms\runtimes\win\lib\net46\System.Security.Cryptography.Algorithms.dll"
refs |> shouldContain @"..\System.Security.Cryptography.Algorithms\lib\netstandard1.6\System.Security.Cryptography.Algorithms.dll"
refs |> shouldNotContain @"..\System.Security.Cryptography.Algorithms\lib\netstandard1.6\System.Security.Cryptography.Algorithms.dll"
refs |> shouldNotContain @"..\System.Security.Cryptography.Algorithms\ref\netstandard1.6\System.Security.Cryptography.Algorithms.dll"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a similar test that checks if the rid stuff works, but now with a lower netstandard?
Because now all those changed to shouldnotcontain here so the test no longer tests that...

@matthid

matthid commented Jun 1, 2017

Copy link
Copy Markdown
Member

Looks good. Travis failure is not related to this PR.
Merging because this fixes the build with MSBuild 15 in a particular nice way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants