-
Notifications
You must be signed in to change notification settings - Fork 10.3k
API docs for HttpAbstractions #26358
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
Conversation
* Arcade suppresses doc errors and warnings by adding it to the NoWarn list. Override this so we get warning from arcade until we can update it. * Finishing touches to API docs for Authentication.Abstractions, Http.Abstractions
src/Http/Authentication.Abstractions/src/IAuthenticationHandler.cs
Outdated
Show resolved
Hide resolved
src/Http/Http.Abstractions/src/Extensions/ResponseTrailerExtensions.cs
Outdated
Show resolved
Hide resolved
@@ -9,6 +9,8 @@ | |||
<PackageTags>aspnetcore;authentication;security</PackageTags> | |||
<IsPackable>false</IsPackable> | |||
<Nullable>enable</Nullable> | |||
|
|||
<NoWarn /> |
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.
@dougbu \ @Pilchie how do you feel about this? Arcade makes 1591 a non-error: https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.DotNet.Arcade.Sdk/tools/ProjectDefaults.props#L94 and this was the easiest way to suppress that behavior.
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.
Seems a big hammer in case there end up being other warnings we need to suppress. An alternative is to add a .editorconfig
file with:
[*.cs]
dotnet_diagnostic.CS1591.severity = error
In any directory where we are ready to enable this, which I think should be considered later than the NoWarn
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.
@wtgodbe suggested doing this instead:
<NoWarn>$(NoWarn.Replace('1591', ''))</NoWarn>
It is much more targeted. Our Directory.Build.props configures this error as a warning: https://github.com/dotnet/aspnetcore/blob/master/Directory.Build.props#L103. If we were to go with your suggestion I guess we would have to say
[*.cs]
dotnet_diagnostic.CS1591.severity = warning
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.
That works too. I think as an end-goal in order to "stay-clean" we'd be better enabling the warning as an error once we don't have any violations.
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.
@scottaddie who's the API ref resource
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.
Our Directory.Build.props configures this error as a warning: https://github.com/dotnet/aspnetcore/blob/master/Directory.Build.props#L103
No, that line says the warning should not be treated as an error. Try with just your $(NoWarn)
update in this project. Should work because project content is loaded after Directory.Build.props and all files it imports
<NoWarn>$(NoWarn.Replace('1591', ''))</NoWarn>
8a449d6
to
a45538b
Compare
Co-authored-by: Chris Ross <[email protected]>
a45538b
to
189054f
Compare
Arcade suppresses doc errors and warnings by adding it to the NoWarn list.
Override this so we get warning from arcade until we can update it.
Finishing touches to API docs for Authentication.Abstractions, Http.Abstractions