Make OrderBy binary compat across TFMs #68036
Conversation
This puts `OrderBy` back into our Workspaces layer. It also addresses a number of other places where our `internal` API was not consistent with `public` API when polyfills were involved. Please consult the markdown doc for the rational here. That is going to be a repo artifact so lets focus the _why_ discussion there. Want it to be clear for future readers why this change is being made.
ghost
added
Area-Compilers
untriaged
|
|
||
| It is not reasonable for us to take the union of all TFM and multi-target every single project to them. That would add several hundred compilations to any build operation which would in turn negatively impact our developer throughput. Instead we attempt to use the TFM where needed. That keeps our builds smaller but increases complexity a bit as we end up shipping a mix of TFM for binaries across our layers. | ||
|
|
||
| # Require consistent API across Target Frameworks |
There was a problem hiding this comment.
@sharwell is looking into an analyzer that can make this automatically enforced vs manually enforced. That will be handled in a different PR. This doc will still serve as a good why that analyzer exists.
|
@dotnet/roslyn-infrastructure, @dotnet/roslyn-compiler PTAL |
jaredpar
removed
the
untriaged
docs/Target Framework Strategy.md
Outdated
|
|
||
| This problem primarily comes from our use of polyfill APIs. To avoid this we employ the following rule: | ||
|
|
||
| > When there is an `#if NET.*` that declares a non-private member, there must be an `#else` that defines an equivalent binary compatible symbol |
There was a problem hiding this comment.
In this case, #if NET.* is a regular expression, but it could be confused as a glob by the reader where . is literal.
There was a problem hiding this comment.
Tried to clean that up a bit.
There was a problem hiding this comment.
Why not use NET... everywhere for consistency?
Co-authored-by: Sam Harwell <[email protected]> Co-authored-by: Chris Sienkiewicz <[email protected]>
|
|
||
| ```csharp | ||
| #if NET7_0_OR_GREATER | ||
| public static IOrderedEnumerable<T> Order<T>(IEnumerable<T> source) where T : IComparable<T> |
There was a problem hiding this comment.
Feels like there's less of a chance of error if we just #if the this keyword, rather than the whole signature.
There was a problem hiding this comment.
I thought that too but it looked really awful when I wrote it that way.
There was a problem hiding this comment.
public static IOrderedEnumerable<T> Order<T>(
#if !NET7_0_OR_GREATER
this
#endif
IEnumerable<T> source) where T : IComparable<T>Yea... idk if I like that
Co-authored-by: Jan Jones <[email protected]> Co-authored-by: Fred Silberberg <[email protected]>
|
Test failure is unrelated. |
8 participants
This puts
OrderByback into our Workspaces layer. It also addresses a number of other places where ourinternalAPI was not consistent withpublicAPI when polyfills were involved.Please consult the markdown doc for the rational here. That is going to be a repo artifact so lets focus the why discussion there. Want it to be clear for future readers why this change is being made.