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

Don't include empty prefix with EnvironmentVariablesConfigurationProvider #89612

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Jul 28, 2023

Don't include an empty prefix for EnvironmentVariablesConfigurationProvider. Makes config debug view cleaner to view:

image

Note that this is a runtime change to ToString. It needs to be modified in ToString because it's used to build up the debug view string.

@ghost
Copy link

ghost commented Jul 28, 2023

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

Issue Details

Don't include an empty prefix for EnvironmentVariablesConfigurationProvider. Makes config debug view cleaner to view:

image

Note that this is a runtime change to ToString. It needs to be modified in ToString because it's used to build up the debug view string.

Author: JamesNK
Assignees: JamesNK
Labels:

area-Extensions-Configuration

Milestone: -

@JamesNK JamesNK changed the title Minor configuration debug changes Don't include empty prefix with EnvironmentVariablesConfigurationProvider Jul 28, 2023
@tarekgh
Copy link
Member

tarekgh commented Jul 28, 2023

Would that be a breaking change for any user calling ToString in non-debugging scenarios?

@tarekgh tarekgh added this to the 8.0.0 milestone Jul 28, 2023
@JamesNK
Copy link
Member Author

JamesNK commented Jul 28, 2023

Maybe, which is why I called it out. However, I don't know why anyone would want to parse the result of ToString. All the configuration provides override ToString for debugging. It's not in an easily parsable format.

@tarekgh
Copy link
Member

tarekgh commented Jul 28, 2023

I don't know why anyone would want to parse the result of ToString.

I understand but nobody knows. Would it make sense if we proceed with this is to file a breaking charge issue? https://github.com/dotnet/docs/issues/new?assignees=gewarren&labels=breaking-change%2CPri1%2Cdoc-idea&projects=&template=breaking-change.yml&title=%5BBreaking+change%5D%3A+

@JamesNK
Copy link
Member Author

JamesNK commented Jul 28, 2023

I don't think we'd create a breaking change issue in dotnet/aspnetcore for this level of change, but dotnet/runtime might have a different standard. It's up to you.

@JamesNK
Copy link
Member Author

JamesNK commented Jul 28, 2023

@tarekgh Ok to merge?

@tarekgh tarekgh merged commit 5735919 into dotnet:main Jul 28, 2023
103 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants