Skip to content
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

Binder gen incremental final #92654

Closed
wants to merge 2 commits into from

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Sep 26, 2023

visualizing to get minimal diff

@layomia layomia added the NO-REVIEW Experimental/testing PR, do NOT review it label Sep 26, 2023
@ghost ghost assigned layomia Sep 26, 2023
@ghost
Copy link

ghost commented Sep 26, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

visualizing to get minimal diff

Author: layomia
Assignees: layomia
Labels:

NO-REVIEW, area-Extensions-Configuration

Milestone: -

@layomia layomia force-pushed the binder-gen-incremental-final branch 8 times, most recently from ebba9ae to 42889ea Compare September 26, 2023 23:08

public static string GetConfigKeyCacheFieldName(ObjectSpec type) => $"s_configKeys_{type.IdentifierCompatibleSubstring}";

public static string GetParseMethodName(ParsableFromStringSpec type)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

9:0 - switch to using type.IdentifierCompatibleSubstring.

Comment on lines +157 to +160
// List<string> is used in generated code as a temp holder for formatting
// an error for config properties that don't map to object properties.
_namespaces.Add("System.Collections.Generic");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

9.0: only add when object has complex members.

"Microsoft.Extensions.Configuration",
};

public BindingHelperInfo ToIncrementalValue()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

9.0: reinstate sorting for equatable arrays where possible e.g. changing property order shouldn't cause regeneration.

@layomia
Copy link
Contributor Author

layomia commented Sep 27, 2023

Merged #89587.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration NO-REVIEW Experimental/testing PR, do NOT review it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant