Fixes mapping for C# tuples with more than seven elements #1242
Conversation
|
@mgravell Would you be able to set my expectations for an ETA on the review? I hate to bug you. |
|
You're right, I need to find the time. I'll see if I can review on Tuesday
(long family weekend here).
…On Sat, 4 May 2019, 23:14 Joseph Musser, ***@***.***> wrote:
@mgravell <https://github.com/mgravell> Would you be able to set my
expectations for an ETA? I hate to bug you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1242 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAEHMHZPHNM2YFNPTNHLQLPTYDEPANCNFSM4HHKOE3A>
.
|
|
I've given it a once over - looks kinda great, which is nice; I'm cautious about the |
|
Awesome! I'll write that nullable tuple test and fix the culture sensitivity either late tonight or sometime tomorrow and ping you when I'm finished. After you get a chance to do the final review, I'll be curious if there's a prerelease feed. |
|
Preview feed on myget via appveyor:
https://www.myget.org/feed/stackoverflow/package/nuget/Dapper
…On Thu, 9 May 2019, 21:26 Joseph Musser, ***@***.***> wrote:
Awesome! I'll write that nullable tuple test and fix the culture
sensitivity either late tonight or sometime tomorrow and ping you when I'm
finished.
After you get a chance to do the final review, I'll be curious if there's
a prerelease feed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1242 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAEHMHLGNZ6OHQWDK3DNHDPUSCFXANCNFSM4HHKOE3A>
.
|
|
@mgravell Ready for the second round! |
|
The force pushes above are adding Do you think you might have a chance coming up next week to review again? I appreciate the time you've been able to give. |
|
@mgravell The workaround at the moment is to introduce new private throwaway structs, so this keeps coming up. Is this PR a very risky one? Would it help if I added more tests? |
|
let's do this... |
|
We'll start using the MyGet feed right away! Thank you so much! 🎉 |
|
No: thank you. It was your contribution, and it is very much appreciated. Delay here was mostly my time juggling. Thanks again. |
|
As an OSS maintainer, I know PRs are also demanding. I appreciate you reviewing and getting this through. |
| il.Emit(OpCodes.Ldarg_0); // stack is now [...][reader] | ||
| EmitInt32(il, index); // stack is now [...][reader][index] | ||
| il.Emit(OpCodes.Dup);// stack is now [...][reader][index][index] | ||
| il.Emit(OpCodes.Stloc_0);// stack is now [...][reader][index] |
…1242) * Failing tests for tuples larger than 7 * Special-case IL generation for ValueTuple * Make value-processing logic available to ValueTuple generation
Fixes #982. I think I made the minimum change possible.
/cc @mgravell