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

Add schema in launchSettings.json files #44295

Merged
merged 4 commits into from
Oct 12, 2022
Merged

Add schema in launchSettings.json files #44295

merged 4 commits into from
Oct 12, 2022

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Sep 30, 2022

Fixes #38120

Complementary to SchemaStore/schemastore#2507

@sebastienros sebastienros requested a review from a team as a code owner September 30, 2022 21:59
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 30, 2022
@JamesNK
Copy link
Member

JamesNK commented Oct 1, 2022

I'm not sure if this is the right approach. appsettings.json, for example, has the schema automatically inferred by VS and VS code.

Change to the VS code extension for that: dotnet/vscode-csharp#4280. There was a similar change in VS.

Do we want the schema added to these files, or should it be consistent with what appsettings.json does?

@sebastienros
Copy link
Member Author

@JamesNK it's nice that the tooling can infer the schema, but I don't think it's contradicting this PR. If someone uses another tool, let's say Rider, or VS for MAC (not sure it does it too). And assuming adding the schema in the templates don't conflict with the tooling.

One template already had the reference, so if we decide to not add the schema then I will remove it from there (

"$schema": "https://json.schemastore.org/launchsettings.json",
)

In any case we can also ponder if launchSettings support should be added to tooling too.

@JamesNK
Copy link
Member

JamesNK commented Oct 1, 2022

Another thing to consider is the schema file that VS generates if one isn't present. I tested deleting the file and restarting VS, and the newly generated launchSettings.json doesn't have a $schema property.

@sebastienros
Copy link
Member Author

Should we just discard this PR then and assume that the issue is fixed as we updated the schema to add more properties?

I see that the changes I pushed on the schema are actually already working in VS, and it's inferred:

image

@JamesNK
Copy link
Member

JamesNK commented Oct 6, 2022

Should we just discard this PR then and assume that the issue is fixed as we updated the schema to add more properties?

Yeah, I think so. Although it sounds like one launchSettings.json is specifying the schema. Remove it from that one for consistency?

@sebastienros
Copy link
Member Author

Found 4 files with it, 2 sample and 2 templates

@sebastienros
Copy link
Member Author

@JamesNK GTG?

@sebastienros
Copy link
Member Author

sebastienros commented Oct 12, 2022

@dotnet/aspnet-build please merge

@sebastienros sebastienros merged commit 1a96a05 into main Oct 12, 2022
@sebastienros sebastienros deleted the sebros/38120 branch October 12, 2022 20:51
@ghost ghost added this to the 8.0-preview1 milestone Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document launchSettings.json
2 participants