close #2636: added json schemas for Aspire.Hosting.AppHost and Aspire.Hosting.Azure#4912
Conversation
eerhardt
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @vladimir-shirmanov!
|
|
||
| <ItemGroup> | ||
| <None Include="**/*.props;**/*.targets" Pack="true" PackagePath="%(RecursiveDir)%(Filename)%(Extension)" /> | ||
| <None Include="**/*.props;**/*.targets;*.json" Pack="true" PackagePath="%(RecursiveDir)%(Filename)%(Extension)" /> |
There was a problem hiding this comment.
| <None Include="**/*.props;**/*.targets;*.json" Pack="true" PackagePath="%(RecursiveDir)%(Filename)%(Extension)" /> | |
| <None Include="**/*.props;**/*.targets" Pack="true" PackagePath="%(RecursiveDir)%(Filename)%(Extension)" /> | |
| <None Include="AspireAppHostConfiguration.json" Pack="true" /> |
Maybe we should just pack the one file we want, that way we don't accidently start including other .json files we don't intend to put in the nupkg.
There was a problem hiding this comment.
sure, fixed it
| <Project> | ||
| <ItemGroup> | ||
| <JsonSchemaSegment Include="$(MSBuildThisFileDirectory)..\AspireAzureConfigurationSchema.json" | ||
| FilePathPattern="appsettings\..*json" /> |
There was a problem hiding this comment.
A lot of times these properties go into "User Secrets", which is a secrets.json file. This is a Regex, so I think we should also add secrets.json to it. @alexgav - did we ever hook up the FilePathPattern correctly in VS?
| FilePathPattern="appsettings\..*json" /> | |
| FilePathPattern="appsettings\..*json|secrets.json" /> |
There was a problem hiding this comment.
I tried to add it, but it didn't work for me. So I skipped for now
| @@ -0,0 +1,6 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
I think we only need a .targets file in buildTransitive. We shouldn't need the other 2 targets files. Just move this logic in there.
There was a problem hiding this comment.
sure, removed all unnecessary ones
| <Compile Include="..\Shared\Cosmos\CosmosConstants.cs" Link="Shared\CosmosConstants.cs" /> | ||
| </ItemGroup> | ||
|
|
||
There was a problem hiding this comment.
ah sorry, removed it as well
|
@eerhardt thank you for the feedback. |
Add some more descriptions and tweak wording.
eerhardt
left a comment
There was a problem hiding this comment.
Nice work @vladimir-shirmanov! Thank you for this contribution. This looks good to me. I'm setting it to 'auto-merge' when CI passes.
Description
fixed an issues with intellisense inside appsettings.json when using Aspire.Hosting.AppHost or Aspire.Hosting.Azure
Changes:
This will close #2636
Microsoft Reviewers: Open in CodeFlow