Skip to content

Conversation

@rstam
Copy link
Contributor

@rstam rstam commented Jan 14, 2026

No description provided.

@rstam rstam requested a review from a team as a code owner January 14, 2026 19:32
@rstam rstam added the feature Adds new user-facing functionality. label Jan 14, 2026
@rstam
Copy link
Contributor Author

rstam commented Jan 14, 2026

Note to reviewers: while reviewing this PR consider not only the fix that was made but also whether there is danger of unintended side effects that we have not detected with our existing tests.

{
throw new ArgumentException($"{typeof(TEnumUnderlyingType).FullName} is not the underlying type of {typeof(TEnum).FullName}.");
}
if (typeof(TIntegralType) != typeof(int) && typeof(TIntegralType) != typeof(long) && typeof(TIntegralType) != typeof(uint) && typeof(TIntegralType) != typeof(ulong))
Copy link
Member

Choose a reason for hiding this comment

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

What about byte?

Copy link
Member

Choose a reason for hiding this comment

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

BTW, can we use IsIntegral method here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


public static bool IsIntegral(this Type type)
{
return type == typeof(int) || type == typeof(long) || type == typeof(uint) || type == typeof(ulong);
Copy link
Member

Choose a reason for hiding this comment

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

What about byte?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified to add all integral types.

@rstam rstam requested a review from sanych-sun January 17, 2026 17:07
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.

2 participants