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 SnippetGenerator to add language hint #22090

Merged
merged 5 commits into from
Jun 23, 2021

Conversation

christothes
Copy link
Member

@christothes christothes commented Jun 23, 2021

This change adds the language="csharp" property to the <code> xml tag and also makes it possible to error on unused snippets.

For unused snippet validation, there is a new switch added to Update-Snippets.ps1 called -StrictMode. Usage is as follows:

The default behavior is to print the names of unused snippets, but not to thow:

 .\eng\scripts\Update-Snippets.ps1 tables
Discovered snippets:
<...>

Not all snippets were used.
Snippet:TablesSample1CreateTableAsync
Snippet:TablesSample1DeleteTableAsync
<...>
Snippet:TablesSample5UpsertWithReplace
Snippet:TablesSample5UpdateEntity
SnippetGenerator completed in 00:00:00.5049468

With the StrictMode switch, it looks like this:

 .\eng\scripts\Update-Snippets.ps1 tables -StrictMode
Discovered snippets:
<...>

System.InvalidOperationException: Not all snippets were used.
Snippet:TablesSample1CreateTableAsync
Snippet:TablesSample1DeleteTableAsync
<...>
Snippet:TablesSample5UpsertWithReplace
Snippet:TablesSample5UpdateEntity
SnippetGenerator completed in 00:00:00.5049468

created the following issues to track remediation of unused snippets https://github.com/Azure/azure-sdk-for-net/search?q=Remove+unused+snippets&type=issues

Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

General and code-owned files LGTM. Curious, though: is this a convention for Microsoft Docs? According to https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/code the language attribute isn't defined. Based on https://docs.microsoft.com/en-us/contribute/code-in-docs it seems that might be the case since there is similar syntax for including snippets from other files, but I wanted to double check.

Assert.AreEqual(expected, reProcessed);
}

private string SnippetProvider(string s) => Processed;
private ValueTask<string> SnippetProvider(string s) => new(Processed);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should use Async suffix.

@pakrym
Copy link
Contributor

pakrym commented Jun 23, 2021

Should we start erroring out for unused snippets? It would prevent us from accidentally breaking the generator and people from using the wrong snippet (like I did very recently).

@christothes
Copy link
Member Author

Should we start erroring out for unused snippets? It would prevent us from accidentally breaking the generator and people from using the wrong snippet (like I did very recently).

What should we do with the unused snippets? Ran locally with the changes and got this:

System.InvalidOperationException: Not all snippets were used.
Snippet:IotHubCreateDeviceIdentities
Snippet:IotHubUpdateDeviceIdentities
Snippet:IotHubUpdateDeviceTwins
Snippet:IotHubDeleteDeviceIdentities
Snippet:IotHubGetDeviceIdentities
Snippet:IotHubGetDeviceTwins
Snippet:IotHubCreateModuleIdentities
Snippet:IotHubUpdateModuleIdentities
Snippet:IotHubUpdateModuleTwins
Snippet:IotHubDeleteModuleIdentities
Snippet:IotHubGetModuleIdentities
Snippet:IotHubGetModuleTwins
Snippet:IotHubCreateConfiguration
Snippet:IotHubGetConfiguration
Snippet:IotHubUpdateConfiguration
Snippet:IotHubDeleteConfiguration
Snippet:IotHubImportJob
Snippet:IotHubExportJob
Snippet:GetImportExportJob
Snippet:IotInvokeMethodOnModule
Snippet:IotInvokeMethodOnDevice
Snippet:IotHubCreateModuleIdentity
Snippet:IotHubGetModuleIdentities
Snippet:IotHubGetModuleIdentity
Snippet:IotHubUpdateModuleIdentity
Snippet:IotHubGetModuleTwin
Snippet:IotHubUpdateModuleTwin
Snippet:IotHubDeleteModuleIdentity
Snippet:IotHubQueryDeviceTwins
Snippet:IotHubQueryModuleTwins
Snippet:IotHubGetDeviceStatistics
Snippet:IotHubGetServiceStatistics

@pakrym
Copy link
Contributor

pakrym commented Jun 23, 2021

What should we do with the unused snippets? Ran locally with the changes and got this:

Delete them or comment out the #region part. They are still in source control, we just need to make sure the team is aware

@christothes christothes force-pushed the chriss/snippetCsharp branch from c8f6203 to 4d0f518 Compare June 23, 2021 21:49
@christothes christothes merged commit 9fe1cf7 into Azure:main Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants