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

Enable nullable: System.Management.Automation.Provider.ICmdletProviderSupportsHelp #14150

Merged

Conversation

powercode
Copy link
Collaborator

Tracking issue: #12631.

@ghost ghost assigned daxian-dbw Nov 19, 2020
@powercode powercode force-pushed the nullable/ICmdletProviderSupportsHelp branch from a1ba215 to 2d1b3c2 Compare November 19, 2020 21:31
@powercode powercode force-pushed the nullable/ICmdletProviderSupportsHelp branch from 2d1b3c2 to 74ee1c5 Compare November 19, 2020 21:33
@ghost ghost added the Review - Needed The PR is being reviewed label Nov 28, 2020
@ghost
Copy link

ghost commented Nov 28, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

/// <summary>
/// This interface needs to be implemented by providers that want users to see
/// provider-specific help.
/// </summary>
#nullable enable

Choose a reason for hiding this comment

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

💭 This seems a bit strange. I would expect to see it before the documentation comment.

@@ -37,7 +39,7 @@ public interface ICmdletProviderSupportsHelp
[SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Maml", Justification = "Maml is an acronym.")]
string GetHelpMaml(string helpItemName, string path);

Choose a reason for hiding this comment

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

The documentation for this method does not specify what a null return value means. I would expect the implementations and callers of this interface to be annotated at the same time as this interface to ensure the pre- and post-conditions are correctly captured.

@SteveL-MSFT SteveL-MSFT added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 15, 2020
@iSazonov
Copy link
Collaborator

/azp run PowerShell-CI-static-analysis

@ghost ghost removed the Review - Needed The PR is being reviewed label May 18, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@iSazonov iSazonov merged commit 94aa68d into PowerShell:master May 18, 2021
@iSazonov iSazonov assigned iSazonov and unassigned daxian-dbw May 18, 2021
@iSazonov iSazonov added this to the 7.2.0-preview.6 milestone May 18, 2021
@powercode powercode deleted the nullable/ICmdletProviderSupportsHelp branch May 25, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants