Make New-ModuleManifest consistent with Update-ModuleManifest #9104
Conversation
src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs
Outdated
Show resolved
Hide resolved
|
@iSazonov I have updated the tests :). |
| /// <summary> | ||
| /// Specify that the module is prerelease. | ||
| /// </summary> | ||
| [Parameter(Mandatory = false)] |
There was a problem hiding this comment.
No need to set defaults.
| [Parameter(Mandatory = false)] | |
| [Parameter] |
Below too.
| /// Specify whether the module requires explicit user acceptance for install/update/save. | ||
| /// </summary> | ||
| [Parameter(Mandatory = false)] | ||
| public SwitchParameter RequireLicenseAcceptance { get; set; } |
There was a problem hiding this comment.
I'd expect that default is true.
@SteveL-MSFT what do you think?
There was a problem hiding this comment.
In anticipation I set the default to true. :)
| @@ -1197,6 +1217,9 @@ private void BuildPrivateDataInModuleManifest(StringBuilder result, StreamWriter | |||
| BuildModuleManifest(result, "ProjectUri", Modules.ProjectUri, ProjectUri != null, () => QuoteName(ProjectUri), streamWriter); | |||
| BuildModuleManifest(result, "IconUri", Modules.IconUri, IconUri != null, () => QuoteName(IconUri), streamWriter); | |||
| BuildModuleManifest(result, "ReleaseNotes", Modules.ReleaseNotes, !string.IsNullOrEmpty(ReleaseNotes), () => QuoteName(ReleaseNotes), streamWriter); | |||
| BuildModuleManifest(result, "Prerelease", Modules.Prerelease, !string.IsNullOrEmpty(Prerelease), () => QuoteName(Prerelease), streamWriter); | |||
There was a problem hiding this comment.
Perhaps we could use nameof() in BuildModuleManifest() like nameof(Prerelease).
It is question only.
/cc @daxian-dbw
There was a problem hiding this comment.
@pougetat Could you please make the change in all calls of BuildModuleManifest()?
There was a problem hiding this comment.
done. I do want to point out there are a few inconsistencies between variable names here and there. For example :
- HelpInfoUri and Modules.HelpInfoURI
- ClrVersion and Modules.CLRVersion
This could be fixed in another PR.
There was a problem hiding this comment.
Feel free to pull new PR or open new tracking issue.
|
@pougetat Please create new issue in PowerShell-Docs repo and reference in the PR description. (Current link points to source issue that is wrong) Thanks for you contribution! |
PR Summary
The purpose of this PR is to make New-ModuleManifest consistent with Update-ModuleManifest by adding
Prerelease,RequireLicenseAcceptance, andExternalModuleDependenciesproperties.PR Context
#8585
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests