Skip to content

Conversation

@rstam
Copy link
Contributor

@rstam rstam commented Jan 14, 2026

No description provided.

@rstam rstam added the feature Adds new user-facing functionality. label Jan 14, 2026
@rstam rstam requested a review from a team as a code owner January 14, 2026 19:50
// static constructor
static EnumerableMethod()
{
#if NET10_OR_GREATER
Copy link
Member

Choose a reason for hiding this comment

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

This #if def does not make any sense to me. We do not have any target defining it, so the code is never going to be executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense if you consider that very soon we will be adding net10 as a target.

Copy link
Member

Choose a reason for hiding this comment

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

Probably, but for now it's a dead-code. Why do we need to add this code to repo NOW?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've came across this question in CSHARP-5665 wanting to use CommonPrefixLength. I decided not to have a dead and untested code, and will note it as a future improvement for net10 target. I suggest doing the same here.

Also we don't have an official date to support net10 yet.

return __where.MakeGenericMethod(tsource);
}

#if !NET10_OR_GREATER
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this code will be always included, as we have no configuration to have NET10_OR_GREATER defined.

@rstam rstam requested a review from sanych-sun January 14, 2026 20:58
@rstam rstam requested a review from BorisDog January 17, 2026 16:33
Copy link
Member

@sanych-sun sanych-sun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@adelinowona adelinowona left a comment

Choose a reason for hiding this comment

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

LGTM

@BorisDog BorisDog removed their request for review January 21, 2026 18:21
Copy link
Contributor

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

Removed myself as a reviewer.

@rstam rstam merged commit 8b40a67 into mongodb:main Jan 23, 2026
31 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Adds new user-facing functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants