-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Xml comments cleanup and improvement #2007
Conversation
…arams; and fix wrongly referenced parameters in comments
Fix SA1629 in ArgumentOutOfRangeException XML comments and an summary tags
Fix SA1611 "The documentation for parameter 'key' is missing"
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2007 +/- ##
=======================================
Coverage 83.64% 83.64%
=======================================
Files 312 312
Lines 7105 7105
Branches 1054 1054
=======================================
Hits 5943 5943
Misses 789 789
Partials 373 373
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were there some warnings you meant to turn back on in this commit? Also I'd have expected there to be some compiler warnings here for some of the changes I've noted that are referencing the wrong/missing parameters.
If I understood you correctly - it would be nice to get compiler warnings about documentation issues. At the moment stylecop documentation warnings are visible only when editing files.
I guess there are no such code-analysis/stylecop documentation rules. Maybe in some other documentation analyzers. |
Yes, I would have expected the compiler to warn about the references to parameters that didn't exist. |
So the reason this isn't happening is because we don't set If I enable that locally for this branch and build the project, then I get 113 warnings: |
Thanks for all the changes so far - if you'd like to do another PR, I suggest one that enables generation of the documentation file as noted above, and then fixing all of the remaining issues together with that to get us down to zero. |
Pull Request
The issue or feature being addressed
Contributes to #1290 by addressing some XML documentation quality issues
Details on the issue fix or feature implementation
Fixes various XML comments quality issues
ArgumentOutOfRangeException
(but these could be further improved)ArgumentNullException
as following/// <exception cref="ArgumentNullException">Thrown when <paramref name="parameter"/> is <see langword="null"/>.</exception>"
Polly/src/Polly.Core/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderExtensions.cs
Line 26 in 1dba402
Confirm the following