Skip to content

Conversation

@dougbu
Copy link
Contributor

@dougbu dougbu commented Feb 25, 2023

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 25, 2023
@dougbu
Copy link
Contributor Author

dougbu commented Feb 25, 2023

Noticed a few issues when glancing at #46882.

The changes contain a few comments for reviewers. Though I used loc comment syntax (start w/ an underscore and end w/ ".comment"), the comments are not intended to be merged.

@sayedihashimi
Copy link
Member

It would be great to have @phenning to review the template related updates here.

Copy link
Member

@phenning phenning left a comment

Choose a reason for hiding this comment

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

From a Visual Studio perspective, LGTM. Note that we don't surface any of the item template metadata directly in VIsual Studio as of now anyway, as we still use the vstemplate -> template engine pattern rather than exposing the sdk template directly in the new item dialog.

@dougbu
Copy link
Contributor Author

dougbu commented Feb 27, 2023

/fyi The Arcade SDK adds a Microsoft.TemplateEngine.Authoring.Tasks package reference in all projects. That detects template.json files and propagate the strings into localize/templatestrings.*.json files.

Copy link
Contributor

@mkArtakMSFT mkArtakMSFT left a comment

Choose a reason for hiding this comment

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

Thanks @dougbu !

@dougbu
Copy link
Contributor Author

dougbu commented Feb 28, 2023

What are your thoughts on the questions I placed in the templatestrings.en.json files @mkArtakMSFT❔ (Not merging this w/o at least removing those comments.)

Copy link
Contributor

@mkArtakMSFT mkArtakMSFT left a comment

Choose a reason for hiding this comment

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

Answered your questions

- follow up to #46714
- align template.json and templatestrings.en.json files
- correct 'ReadWrite' and 'Am'
- ask a few questions of reviewers
- no need to change post-action `description`s
- remove comment nodes from templatestrings.en.json files
- also, update English resources to match template.json content
- fill in missed files in localize/ folder
@dougbu dougbu enabled auto-merge (squash) February 28, 2023 19:51
@dougbu dougbu merged commit a580b5a into dotnet:main Feb 28, 2023
@dougbu dougbu deleted the dougbu/spelling branch February 28, 2023 21:45
@ghost ghost added this to the 8.0-preview3 milestone Feb 28, 2023
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.

5 participants