Skip to content

Conversation

@T-Gro
Copy link
Member

@T-Gro T-Gro commented Dec 21, 2022

Addresses #14398

  • Make a new FsharpType.MakeStructTupleType overload that does not accept an assembly argument

  • Obsolete existing members in reflection and quotations that still need it

  • Change the inner implementation to be lock-free and work with netstandard ref-tuples and struct-tuples

  • Add new tests (e.g. for 'tuple nesting' behaviour)

  • Inner workings of Obsolete code moved to a separate module, to make them easy to delete once API surface can be reduced

@T-Gro T-Gro requested a review from a team as a code owner December 21, 2022 15:00
@vzarytovskii
Copy link
Member

Since it's a breaking change (existing method is getting Obsolete'd), please, bump FSharp.Core version.
@KevinRansom will minor version be enough?

@abelbraaksma
Copy link
Contributor

Just curious, the change makes total sense, but is there a tracking issue or is this just general cleanup? Since we stopped updating the changelog (I’d love to have that back, but that’s OT), in particular with backward compat issues, having a tracking issue would be nice.

@T-Gro
Copy link
Member Author

T-Gro commented Dec 27, 2022

Just curious, the change makes total sense, but is there a tracking issue or is this just general cleanup? Since we stopped updating the changelog (I’d love to have that back, but that’s OT), in particular with backward compat issues, having a tracking issue would be nice.

Only this:
#14398

@T-Gro
Copy link
Member Author

T-Gro commented Dec 27, 2022

You meant a tracking issue for all API-relevant-changes to be linked into, right?

@T-Gro T-Gro requested review from a team, 0101, KevinRansom and vzarytovskii December 27, 2022 09:50
@abelbraaksma
Copy link
Contributor

abelbraaksma commented Dec 28, 2022

Yes, that’s what I meant. Usually it’s referenced in the PR description if there is one (and afaik, you usually link it), hence me wondering ;).

@T-Gro
Copy link
Member Author

T-Gro commented Dec 29, 2022

Yes, that’s what I meant. Usually it’s referenced in the PR description if there is one (and afaik, you usually link it), hence me wondering ;).

Understood, just not seeing an appropriate one.
@vzarytovskii : Can you help me out please? Looking for a meta-issue where we could collected implemeneted && upcoming API-relevant changes/additions.

@abelbraaksma
Copy link
Contributor

Actually, I just meant linking #14398. But you’re right, a tracking issue to list API changes is something we’ve used in the past and would be really useful.

@T-Gro
Copy link
Member Author

T-Gro commented Jan 6, 2023

Deprecation via [<Obsolete>] removed, only new APIs added.
Please re-review.

KevinRansom
KevinRansom previously approved these changes Jan 9, 2023
Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Minor stuff thanks.

@T-Gro T-Gro self-assigned this Jan 9, 2023
@T-Gro T-Gro added this to the January-2023 milestone Jan 9, 2023
@T-Gro T-Gro enabled auto-merge (squash) January 10, 2023 07:50
@T-Gro T-Gro requested a review from KevinRansom January 12, 2023 10:12
@T-Gro T-Gro merged commit 9cd394e into main Feb 2, 2023
@T-Gro T-Gro deleted the 14398-add-fsharptypemakestructtupletype-overload-that-does-not-accept-an-assembly branch February 2, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Add FSharpType.MakeStructTupleType overload that does not accept an Assembly.

8 participants