-
Notifications
You must be signed in to change notification settings - Fork 4k
Remove all XML help #3655
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
Remove all XML help #3655
Conversation
…script to build.proj
a593485
to
17d0618
Compare
@azuresdkci test this please |
…-Help.xml files that do not exist
@markcowl resolved issue with wxi file |
Sign job: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-sign/938/ Update: sign job is passing 👍 |
…t being thrown during validation
@@ -1,319 +0,0 @@ | |||
# PlatyPS Help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure to add in the new help documentation
} | ||
} | ||
|
||
private void ServiceManagementAnalyze( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<nit>AnalyzeMamlHelp</nit>
} | ||
else | ||
{ | ||
ResourceManagerAnalyze(directory, helpLogger, processedHelpFiles, savedDirectory); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<nit>AnalyzeMarkdownHelp</nit>
tools/GenerateHelp.ps1
Outdated
|
||
# --------------------------------------------------------------------------------------------- | ||
|
||
if ($ValidateMarkdownHelp) | ||
{ | ||
New-Item -Path "$PSScriptRoot\HelpGeneration\Exceptions" -Name NewValidateHelpExceptions.csv -ItemType File -Force | Out-Null | ||
|
||
Add-Content "$PSscriptRoot\HelpGeneration\Exceptions\NewValidateHelpExceptions.csv" "Module,Target,Description" | ||
Add-Content "$PSScriptRoot\HelpGeneration\Exceptions\NewValidateHelpExceptions.csv" "Module,Target,Description" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, Exceptions are for expected errors, I think the error csvs are being copied to the build directory, and we should mnake sue of thsi same location Also, do we have any mechanism for suppressions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few requested changes. Otherwise LGTM
<Message Importance="high" Text="Running Static Analyser" /> | ||
<CallTarget targets="DependencyAnalysis" /> | ||
<Exec Command="$(PowerShellCommand) -NonInteractive -NoLogo -NoProfile -Command ". $(LibraryToolsFolder)\CleanupBuild.ps1 -BuildConfig $(Configuration) ""/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make sure the build fails if this fails? We can do this by settign ContinueOnError="false", similar to this: https://github.com/Azure/azure-powershell/blob/stack-dev/build.proj#L369
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, lets make sure that building the installer always happens after generating help
@@ -462,9 +462,6 @@ | |||
<Component Id="cmp6BDCB170E1F314882CF58F8F80DE76A6" Guid="*"> | |||
<File Id="filBFDE1699047559AB8DA89E0F660D35DF" KeyPath="yes" Source="$(var.sourceDir)\ResourceManager\AzureResourceManager\AzureRM.Billing\Microsoft.Azure.Commands.Common.Authentication.dll" /> | |||
</Component> | |||
<Component Id="cmpED253190CB7DAF0A995567435152DC6A" Guid="*"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should not be deleted. We need the xml help files in the installer, or there will be no online help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markcowl all dll-Help.xml references are still in the wxi file - the ones being removed are from projects with dependencies on other projects (e.g., Profile, Tags, Resources)
tools/GenerateHelp.ps1
Outdated
Param( | ||
[Parameter()] | ||
[Switch]$ValidateMarkdownHelp, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<nit>
remove extra veritcal space. You should only have a vertical space after a closing brace }
, or the like </nit>
{ | ||
$HelpFolder = Get-Item $HelpFolderPath | ||
|
||
$Exceptions = Import-Csv "$PSScriptRoot\..\..\src\Package\Exceptions\ValidateHelpExceptions.csv" | where { $_.Module -eq "$($HelpFolder.Parent.Name)" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a parameter
# If there were any errors recorded, print them out and throw | ||
if ($errors.Count -gt 0) | ||
{ | ||
$errors | foreach { Add-Content "$PSScriptRoot\..\..\src\Package\ValidateHelpExceptions.csv" $_ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this outupe file should be a parameter. Also, I think you want this under src\Package\{buildconfig}
by default in order to get it included in the build. Finally, this should be called ValdateHelpIssues.csv to be consistent with the other products of static analysis.
@@ -0,0 +1,343 @@ | |||
#Requires -Modules platyPS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about extra vertical space, this applies throughout
…nto remove-xml-help # Conflicts: # src/ResourceManager/Compute/Commands.Compute/Commands.Compute.csproj # src/ResourceManager/Network/Commands.Network/Microsoft.Azure.Commands.Network.dll-Help.xml # src/ResourceManager/Sql/Commands.Sql/help/New-AzureRmSqlElasticPool.md
@azuresdkci test this please |
…nto remove-xml-help # Conflicts: # src/ResourceManager/Storage/Commands.Management.Storage/Microsoft.Azure.Commands.Management.Storage.dll-Help.xml # src/ResourceManager/Storage/Commands.Management.Storage/help/New-AzureRmStorageAccount.md # src/ResourceManager/Storage/Commands.Management.Storage/help/Set-AzureRmStorageAccount.md
…e-powershell into remove-xml-help
Description
Fix for issue #3583
This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.
General Guidelines
Testing Guidelines
Cmdlet Signature Guidelines
ShouldProcess
and haveSupportShouldProcess=true
specified in the cmdlet attribute. You can find more information onShouldProcess
here.OutputType
attribute if any output is produced - if the cmdlet produces no output, it should implement aPassThru
parameter.Cmdlet Parameter Guidelines