Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

[release/1.1.0] Inbox value tuple #16447

Merged
merged 4 commits into from
Mar 22, 2017
Merged

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Feb 24, 2017

Require buildtools update to build w/o validation errors.
dotnet/buildtools#1348

Fixes: #14235
/cc @jcouv @marek-safar @weshaggard

@jcouv
Copy link
Member

jcouv commented Feb 24, 2017

ValueTuple is only inbox after a certain version on mono. Should the change be conditional on the version of mono?

@ericstj
Copy link
Member Author

ericstj commented Feb 24, 2017

This doesn't apply for Mono, that behaves as desktop so its stuck getting the desktop binary.

Xamarin doesn't version their TFMs in a way that lets you target the framework API so those frameworks are stuck getting the behavior of the latest SDKs.

@ericstj ericstj added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 24, 2017
@ericstj
Copy link
Member Author

ericstj commented Feb 24, 2017

Marking this as NO-MERGE since we need to wait for 1.1.0 to exit lock down and need to add the build-tools update. This is somewhat dependent on #16119 which does the boiler-plate work to version the ValueTuple package for servicing.

@ericstj
Copy link
Member Author

ericstj commented Mar 1, 2017

Now that #16119 is not going in we need to bring over the versioning changes to this PR.

@ericstj
Copy link
Member Author

ericstj commented Mar 1, 2017

Brought those commits over. I expect this to fail during package validation until we can get dotnet/buildtools#1348

@ericstj ericstj removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 15, 2017
@ericstj
Copy link
Member Author

ericstj commented Mar 15, 2017

This is ready to go, @jcouv and @marek-safar can you guys please try "System.ValueTuple": "4.3.1-servicing-25101-0" from feed https://www.myget.org/F/ericstj-staging/api/v3/index.json?

@jcouv
Copy link
Member

jcouv commented Mar 15, 2017

I've run the tests from ValueTupleTests.cs against the package in a .Net Framework 2.0 project (with XUnit 1.9). I think that covers the PCL portion of the package.

I'm now trying to do the same with the .Net Standard portion of the package. But I'm having trouble getting XUnit to work on .Net Standard 1.0 or 1.1. I'll keep you posted.

@ericstj
Copy link
Member Author

ericstj commented Mar 16, 2017

@marek-safar / @mhutch / @akoeplinger can one of you ack this and do whatever testing you need to make sure it satisfies your ask? Given this fix is specifically for Xamarin TFMs I want to make sure it gets tested there.

@jcouv
Copy link
Member

jcouv commented Mar 16, 2017

Thanks for the tips @ericstj
I've successfully run all the tests from ValueTupleTests.cs in a .Net Standard 1.0 project as well.

@jcouv
Copy link
Member

jcouv commented Mar 20, 2017

@marek-safar / @mhutch / @akoeplinger Can you validate the change with mono? Thanks in advance.

@marek-safar
Copy link
Contributor

@ericstj @jcouv I can confirm that this package works as expected (no compiler errors when System.ValueTuple is used) on Xamarin dev15-2

@stephentoub
Copy link
Member

@ericstj, can this be merged now?

@ericstj
Copy link
Member Author

ericstj commented Mar 22, 2017

Let me double check with ship-room on that. We had early approval before the buildtools changes went in I just want an ACK that we're OK to merge.

@ericstj
Copy link
Member Author

ericstj commented Mar 22, 2017

We have the green light. Merging

@ericstj ericstj merged commit 27f57f5 into dotnet:release/1.1.0 Mar 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants