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

Update appsettings schema with Kestrel and Logging configuration. #1386

Merged

Conversation

JamesNK
Copy link
Contributor

@JamesNK JamesNK commented Dec 8, 2020

Work in progress. Temporarily removed existing properties.

@JamesNK
Copy link
Contributor Author

JamesNK commented Dec 8, 2020

@Tratcher @halter73 @davidfowl Please review the options and descriptions. I want to make sure everything is included and the comments and defaults are accurate.

There is no documentation for some of this (e.g. SNI in config) so I had to look at the code and figure out how it works.

src/schemas/json/appsettings.json Outdated Show resolved Hide resolved
src/schemas/json/appsettings.json Outdated Show resolved Hide resolved
src/schemas/json/appsettings.json Show resolved Hide resolved
src/schemas/json/appsettings.json Outdated Show resolved Hide resolved
src/schemas/json/appsettings.json Outdated Show resolved Hide resolved
@JamesNK
Copy link
Contributor Author

JamesNK commented Dec 12, 2020

@madskristensen The existing root properties in the schema use patternProperties, which started when you added the first version. I was wondering why this is done, and whether the common .NET properties I'm adding in this PR should do the same.

My theories are either:

  1. patternProperties doesn't show up in autocomplete. This is desirable for properties like webOptimizer and elmah that aren't in most apps
  2. Case-insensitivity

Should Logger/Kestrel/ConnectionStrings change to pattern properties?

@madskristensen
Copy link
Contributor

@JamesNK Your first theory is the correct one. It was done such that optional 1st and 3rd party components could light up but wasn't being suggested by IntelliSense since the app might not use them at all. So, it was to avoid false positives but still have rich support for any component that wanted IntelliSense in appsettings.json.

@JamesNK JamesNK force-pushed the jamesnk/appsettings-aspnet branch from 7328958 to 7d6b98e Compare December 14, 2020 02:05
@JamesNK
Copy link
Contributor Author

JamesNK commented Dec 14, 2020

@madskristensen Good to merge?

@madskristensen madskristensen merged commit 218cb59 into SchemaStore:master Jan 4, 2021
@madskristensen
Copy link
Contributor

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants