Fix docs comments in utility folder by iSazonov · Pull Request #7192 · PowerShell/PowerShell

iSazonov · 2018-06-27T04:33:36Z

PR Summary

The PR contains only style changes outside codes. It does not aim to eliminate all style issues - only reduce their number and increase the CodeFactor usefulness.

Changes are distributed by commits to make it easier to review.

Add final dot in xml doc comments.
Start comments with capical letter.
Remove empty lines in xml doc comments.
Add new line at EOF.
Remove unneeded comments.
Some minor fixes.

PR Checklist

dantraMSFT · 2018-06-27T23:29:35Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs

        /// <param name="endOfRecord">
-        /// this is true if end of record is reached
-        /// when delimiter is hit. This would be true if delimiter is NewLine
+        /// This is true if end of record is reached


I notice in some cases, you merged multiple lines to have a sentence on a single line but in others, such as here, you don't. Is there any particular reason for this?

I have not set a goal to fix all the problems since there are too many. The main goal is to remove as many problems as possible to kontribjutery not diverted on these messages.

In this case, the IDE performs the formatting themselves when they show this pop-up comment. So deleting strings is better. On the other hand we need to see them on a screen width of 80-120 characters. Although I could miss something-I did not try to catch all the cases.

dantraMSFT · 2018-06-27T23:40:46Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/CustomSerializationStrings.cs

Change case: ConvertTo-XML instead of Convertto-XML

dantraMSFT · 2018-06-27T23:54:01Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs

The period doesn't belong here; it breaks the sentence.

dantraMSFT · 2018-06-28T00:13:38Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/out-file/Out-File.cs

Suggest One-time instead of one time.

dantraMSFT · 2018-06-28T00:16:40Z

...soft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/out-printer/out-printer.cs

Suggest One-time instead of One time.

dantraMSFT · 2018-06-28T17:55:44Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/trace/SetTracerCommand.cs

Suggest: remove the empty comments

dantraMSFT · 2018-06-28T17:57:27Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/trace/TraceExpressionCommand.cs

Remove added period' it breaks the sentence.

dantraMSFT · 2018-06-28T18:00:18Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/update-list.cs

Suggest: change 'to be add' to ' to add'

dantraMSFT · 2018-06-28T18:02:21Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/write.cs

Suggest: I believe these two lines should be one sentence..

...this is ErrorRecord.ErrorDetails.Message; otherwise, the

dantraMSFT · 2018-06-28T18:02:55Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/write.cs

Suggest the same as above.... ErrorDetails.Message; otherwise, ...

SteveL-MSFT · 2018-06-28T21:28:43Z

I looked at some of the changes and my primary concern is that adding a period to the end of a

comment doesn't make it a sentence. Many of the existing comments are just clauses and not complete sentences and adding the period addresses the CodeFactor rule, but makes the comment worse. If we want to address those, we should have better comments that are complete sentences.

iSazonov · 2018-06-29T04:51:21Z

Unfortunately, most public comments are formal and do not do any benefits. We could remove them at all or replace them with a placeholder.

Can someone replace all these formal comments with useful ones? It seems not real. So we can just remove the noisy CodeFactor issues formally.

Let's do a formal review - we can't replace tons of formal comments with meaningful ones.

iSazonov · 2018-06-29T05:48:57Z

@dantraMSFT Your comments was addresed.

SteveL-MSFT · 2018-06-29T16:04:16Z

@iSazonov looking at some of the original comments in the code, they don't provide value other than get past the compiler error of not having a <summary/>. It seems that we would eventually want good comments so that we can generate SDK docs from the code, but that is a non-trivial task to review all the comments and make them useful and complete sentences. Perhaps we should defer that as a separate issue.

anmenaga · 2018-07-13T18:53:08Z

@dantraMSFT please take a look again.

dantraMSFT · 2018-07-13T19:46:42Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs

Capitalize implementation.

dantraMSFT · 2018-07-13T21:03:42Z

...owerShell.Commands.Utility/commands/utility/FormatAndOutput/common/WriteFormatDataCommand.cs

Capitalize type

dantraMSFT · 2018-07-13T21:04:57Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/out-file/Out-File.cs

Capitalize handle

dantraMSFT · 2018-07-13T21:11:22Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/Send-MailMessage.cs

change option to options (plural)

dantraMSFT · 2018-07-13T21:12:48Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ShowCommand/ShowCommand.cs

Until is mispelled.

dantraMSFT · 2018-07-13T21:14:20Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/StartSleepCommand.cs

match the spelling of the Stopprocessing comment with the member name

dantraMSFT · 2018-07-13T22:05:49Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/XmlCommands.cs

Remove the empty comment line.

dantraMSFT · 2018-07-13T22:12:42Z

...owerShell.Commands.Utility/commands/utility/FormatAndOutput/out-printer/PrinterLineOutput.cs

use 'one-time' instead of 'one time'

iSazonov · 2018-07-16T04:46:15Z

@dantraMSFT Your comment was addressed.

dantraMSFT

There are 3 or 4 items that were missed. Search for ping)
Other than that, LGTM

iSazonov · 2018-07-17T03:27:48Z

I corrected one pass, the rest well disguised by GitHub. I suppose we can merge as is.
Thanks for the review @dantraMSFT !

iSazonov added 13 commits June 27, 2018 09:09

Fix xml docs 1

c285ab5

Fix xml docs 2

40d2a06

Fix xml docs 3

b860dfd

Fix xml docs 4

b595681

Fix xml docs 5

08215d3

Fix xml docs 6

08b3044

Fix xml docs 7

8dea90b

Fix xml docs 8

f6e2639

Fix xml docs 9

6013205

Fix xml docs 10

4845d65

Fix xml docs 11

9b9be92

Fix xml docs 12

1ef72da

Fix xml docs 13

4c67f4b

iSazonov requested review from JamesWTruher, PaulHigin and dantraMSFT as code owners June 27, 2018 04:33

iSazonov requested review from SteveL-MSFT and markekraus June 27, 2018 04:34

iSazonov self-assigned this Jun 27, 2018

dantraMSFT suggested changes Jun 28, 2018

View reviewed changes

iSazonov added 2 commits June 29, 2018 10:51

Address dantraMSFT's comments

42ec540

Address some CodeFactor doc issues.

b1ba2d8

dantraMSFT reviewed Jul 13, 2018

View reviewed changes

dantraMSFT suggested changes Jul 13, 2018

View reviewed changes

s dantraMSFT's comments 2

43d3206

iSazonov force-pushed the fix-docs-comments-in-utility branch from 36cb6e2 to 43d3206 Compare July 16, 2018 04:40

Merge branch 'master' into fix-docs-comments-in-utility

d5b7a4e

dantraMSFT approved these changes Jul 16, 2018

View reviewed changes

Use "One-time"

c904baa

iSazonov merged commit a378615 into PowerShell:master Jul 17, 2018

iSazonov deleted the fix-docs-comments-in-utility branch July 17, 2018 05:07

Conversation

iSazonov commented Jun 27, 2018 • edited by TravisEz13 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented Jun 28, 2018

Uh oh!

iSazonov commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Jun 29, 2018

Uh oh!

SteveL-MSFT commented Jun 29, 2018

Uh oh!

anmenaga commented Jul 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

iSazonov commented Jun 27, 2018 •

edited by TravisEz13

Loading

iSazonov commented Jun 29, 2018 •

edited

Loading