Conversation
There was a problem hiding this comment.
We can't accept any change that requires you touching the PublicAPI.Shipped.txt files, as those would be breaking changes.
You would need to suppress any warnings generated instead.
bdd04b5 to
9720433
Compare
|
Ok, I've disabled this rule only for impacted classes in |
.editorconfig
Outdated
| indent_size = 2 | ||
| tab_width = 2 | ||
|
|
||
| # S3872 // Interfaces should not be empty |
There was a problem hiding this comment.
This doesn't appear to be the right description for what the original change was?
It's better (IMHO) to suppress these in the code with #pragma warning disable/restore pairs at the specific points of usage, rather the hiding it away in the editorconfig.
There was a problem hiding this comment.
You are right, in this case we don't have many violations for this rule, so it makes sense to keep it in code. Also, it can serve as a reminder to fix it in the future.
src/Polly/Timeout/TimeoutSyntax.cs
Outdated
| /// <param name="timeout">The timeout.</param> | ||
| /// <returns>The policy instance.</returns> | ||
| /// <exception cref="ArgumentOutOfRangeException">timeout;Value must be a positive TimeSpan (or Timeout.InfiniteTimeSpan to indicate no timeout).</exception> | ||
| #pragma warning disable S3872 |
There was a problem hiding this comment.
The build failures are annoying - I've seen them before where dotnet format doesn't complain locally but does in CI. I think to fix them you need to move the disable to above the /// comments.
There was a problem hiding this comment.
Thanks, works like a charm now.
|
Thanks again! |
Pull Request
The issue or feature being addressed
#1290
Details on the issue fix or feature implementation
Confirm the following