FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix coding style for NewModuleManifestCommand by pougetat · Pull Request #9134 · PowerShell/PowerShell · GitHub

/ PowerShell Public

Fix coding style for NewModuleManifestCommand - #9134

Merged
Ilya (iSazonov) merged 2 commits into
PowerShell:masterfrom
pougetat:clean-newmodulemanifest
Mar 26, 2019
ConversationCommits (2)ChecksFiles changed
Merged

Fix coding style for NewModuleManifestCommand#9134
Ilya (iSazonov) merged 2 commits into
PowerShell:masterfrom
pougetat:clean-newmodulemanifest

Conversation

pougetat (pougetat) commented Mar 14, 2019 •
edited by iSazonov
Loading

Copy link
Copy Markdown

The purpose of this PR is to fix some of the coding style in NewModuleManifestCommand

  • Comments for members
  • 3 member names have been changed so that the use of nameof can be consistent throughout the file : nameof(Modules.CLRVersion) => nameof(CLRVersion). Unfortunately this means not following certain conventions for these 3 member names.

PR Checklist

Ilya (iSazonov) reviewed Mar 23, 2019
View reviewed changes
Comment thread src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs Outdated
/// </summary>
[Parameter]
public Guid Guid
public Guid GUID

Ilya (iSazonov) Mar 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please revert - we should use camel style. Also it is public API (breaking change).

pougetat (pougetat) Mar 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done

Comment thread src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs Outdated
[AllowNull]
[SuppressMessage("Microsoft.Design", "CA1056:UriPropertiesShouldNotBeStrings")]
public string HelpInfoUri
public string HelpInfoURI

Ilya (iSazonov) Mar 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please revert - it is public API (breaking change).

pougetat (pougetat) Mar 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done

Comment thread src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs
public Guid Guid
public Guid GUID
{
get { return _guid; }

Ilya (iSazonov) Mar 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

pougetat (@pougetat) You could review all such unneeded private fields and remove them. Better in follow PR.

pougetat (pougetat) Mar 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

sounds good

Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Mar 23, 2019
Ilya (iSazonov) added this to the 6.3.0-preview.1 milestone Mar 26, 2019
Ilya (iSazonov) self-assigned this Mar 26, 2019
Ilya (iSazonov) merged commit 0f429f2 into PowerShell:master Mar 26, 2019
Rob Holt (rjmholt) mentioned this pull request Jan 18, 2020
Merged
14 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Fix coding style for NewModuleManifestCommand (PowerShell#9134)
5cfcc21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Ilya (iSazonov) iSazonov approved these changes
Bruce Payette (BrucePay) Awaiting requested review from BrucePay
Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw

Assignees

Ilya (iSazonov)

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

7.0.0-preview.1

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL