Remove empty xml comment lines #7356
Conversation
iSazonov
requested review from
SteveL-MSFT,
TravisEz13,
adityapatwardhan and
dantraMSFT
iSazonov
requested review from
BrucePay,
Francisco-Gamino,
JamesWTruher,
PaulHigin,
anmenaga,
daxian-dbw and
mirichmo
as code owners
SteveL-MSFT
left a comment
SteveL-MSFT
left a comment
There was a problem hiding this comment.
I spent a bit of time up through the 4th commit. My main feedback is that where there is content between xml tags, it seems we should preserve the whitespace as it makes reading the comment easier. I'm ok with removing the whitespace between different xml tags.
| /// can subsequently use to create one or more CimSession connections. The | ||
| /// options object holds the CIM Session information that is less commonly set | ||
| /// and used by the IT Pro, and most commonly defaulted. | ||
| /// |
There was a problem hiding this comment.
It seems that this line makes it more readable as there's two paragraphs
There was a problem hiding this comment.
There are many such places. Since I used automatic replacement, it is really impossible to fix it everywhere.
There was a problem hiding this comment.
Is it possible to craft a regex so that if the content of the last newline doesn't end with > then keep the whitespace?
There was a problem hiding this comment.
Yes, I close the PR and open new one #7401 with the new regex.
| /// MshManagementMshSnapin (or MshManagementMshSnapinInstaller) is a class for facilitating registry | ||
| /// of necessary information for monad management mshsnapin. | ||
| /// | ||
| /// This class will be built with monad management dll |
There was a problem hiding this comment.
We should just remove this line, doesn't add value
| /// MshUtilityMshSnapin (or MshUtilityMshSnapinInstaller) is a class for facilitating registry | ||
| /// of necessary information for monad utility mshsnapin. | ||
| /// | ||
| /// This class will be built with monad utility dll |
| /// | ||
| /// The exit code for the shell. | ||
| /// | ||
| /// NTRAID#Windows OS Bugs-1036968-2005/01/20-sburns The behavior here is related to monitor work. The low word of the |
There was a problem hiding this comment.
Remove "NTRAID#Windows OS Bugs-1036968-2005/01/20-sburns ". NTRAID is a really old bug tracking system that is no longer running so no way to open this bug.
There was a problem hiding this comment.
Removed.
There are many such comments so all comments reviewed and fixed in the file.
| /// | ||
| /// NTRAID#Windows OS Bugs-1036968-2005/01/20-sburns The behavior here is related to monitor work. The low word of the | ||
| /// exit code is available for the user. The high word is reserved for the shell and monitor. | ||
| /// |
| /// - if ctrl-break is pressed, with 0xFFFE0000 | ||
| /// - if the shell.exe is passed a bad command-line parameter, with 0xFFFD0000. | ||
| /// - if the shell.exe crashes, with 0x00000000 | ||
| /// |
| /// possibly due to an error). If not, the shell process crashed. If the shell.exe exit code is x00000000 (crashed) | ||
| /// or 0xFFFE0000 (user hit ctrl-break), the monitor should restart the shell.exe. Otherwise, the monitor should exit | ||
| /// with the same exit code as the shell.exe. | ||
| /// |
| } | ||
|
|
||
| /// <summary> | ||
| /// |
There was a problem hiding this comment.
We have tons empty comments - I can not add useful comments everywhere.
There was a problem hiding this comment.
We can treat this as a separate issue outside of this PR
|
New PR #7401 |
2 participants
PR Summary
Please review commit by commit.
with
was reverted because there is no benefits.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests