Skip to content

[feature/10.0] Replace reflection-based options mapping logic #8233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: feature/10.0
Choose a base branch
from

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented May 12, 2025

This replaces the reflection-based logic from CommonOptionsExtensions.cs with a statically-typed version.
The statically typed version could be source generated, but for now it is written by hand (with plenty help from copilot).

CommonOptionsExtensions has been moved into the test tree, and a new testcase has been added that:

  • uses reflection to fully populate a RootOptions instance
  • maps it to a dictionary using both the old and the new logic
  • checks that the results are equal

Includes a fix to the mapping logic to treat Uri as a built-in type (calling ConvertUtils.ToString instead of recursing into its properties).

@sbomer sbomer requested a review from jander-msft May 12, 2025 17:31
@sbomer sbomer requested a review from a team as a code owner May 12, 2025 17:31
@sbomer sbomer changed the title Replace reflection-based options mapping logic [feature/10.0] Replace reflection-based options mapping logic May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant