-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Port .NET 7.0 Preview1 documentation #7807
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
xml/System.Security.Cryptography.X509Certificates/X509Certificate2.xml
Outdated
Show resolved
Hide resolved
xml/System.Security.Cryptography.X509Certificates/X509Certificate2Collection.xml
Outdated
Show resolved
Hide resolved
xml/System.Security.Cryptography.X509Certificates/X509Certificate2Collection.xml
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
@@ -1406,6 +1406,8 @@ This member is an explicit interface member implementation. It can be used only | |||
<exception cref="T:System.InvalidOperationException">The underlying collection was modified outside of this <see cref="T:System.Collections.Concurrent.BlockingCollection`1" /> instance, or the <see cref="T:System.Collections.Concurrent.BlockingCollection`1" /> is empty and the collection has been marked as complete for adding.</exception> | |||
<related type="Article" href="/dotnet/standard/collections/thread-safe/">Thread-Safe Collections</related> | |||
<related type="Article" href="/dotnet/standard/collections/thread-safe/blockingcollection-overview">BlockingCollection Overview</related> | |||
<exception cref="T:System.OperationCanceledException">The <see cref="T:System.Collections.Concurrent.BlockingCollection`1" /> is empty and has been marked | |||
as complete with regards to additions.</exception> |
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.
This doesn't look right. The docs correctly cite this same condition just a few lines above as being for an InvalidOperationException. It's possible the docs were correctly updated and the source just never was? (I'm looking forward to a time when these are maintained in only one place.)
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.
I see what you mean. This is the text we have in triple slash:
/// <exception cref="System.OperationCanceledException">The <see
/// cref="System.Collections.Concurrent.BlockingCollection{T}"/> is empty and has been marked
/// as complete with regards to additions.</exception>
/// <exception cref="System.ObjectDisposedException">The <see
/// cref="System.Collections.Concurrent.BlockingCollection{T}"/> has been disposed.</exception>
/// <exception cref="System.InvalidOperationException">The underlying collection was modified
/// outside of this <see
/// cref="System.Collections.Concurrent.BlockingCollection{T}"/> instance.</exception>
Should I revert this added exception? I am not convinced this API throws OperationCanceledException
anywhere. If yes, then I think the triple slash exceptions should be updated as well.
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.
Should I revert this added exception?
Yes, please.
If yes, then I think the triple slash exceptions should be updated as well.
Yes, the /// comments are incorrect. Not just on this overload but on others as well.
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.
It seems this is the only location that needs fixing in this PR. There's only one other Take
overload, which takes a CancellationToken
argument, and it can throw OperationCanceledException
when it internally calls TryTakeWithNoTimeValidation
. It throws here and here.
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.
and it can throw OperationCanceledException when it internally calls TryTakeWithNoTimeValidation. It throws here and here.
Yes, but the docs on that overload are wrong:
https://github.com/dotnet/runtime/blame/60f593b4dc3760268956b9a874daad7f49b0255d/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/BlockingCollection.cs#L546-L549
It can throw an OperationCanceledException, but not because the collection "is empty and has been marked as complete with regards to additions."
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.
Opened issue to track the triple slash removal: dotnet/runtime#66374
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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.
System.IO and Microsoft.Extensions.Caching.Memory LGTM, thank you @carlossanlop !
This PR contains all the documentation added in .NET 7 Preview1. The changes on each namespace were added in separate commits to make it easier to review.
Area owners, please help review the documentation got ported properly, has no typos, no unexpected xml elements or markdown items. If you find any problems, it would be extremely appreciated if you provide a comment with a code suggestion for easier fixing.
Need review from:
@dotnet/area-extensions-caching
@dotnet/area-extensions-configuration
@dotnet/area-extensions-dependencyinjection
@dotnet/area-extensions-primitives
@dotnet/area-system-buffers
@dotnet/area-system-collections
@dotnet/area-system-diagnostics-activity
@dotnet/area-system-diagnostics-eventlog
@dotnet/area-system-diagnostics-performancecounter
@dotnet/area-system-diagnostics-tracesource
@dotnet/area-system-drawing
@dotnet/area-system-io
@dotnet/ncl
@dotnet/area-system-numerics (also please take a look at some changes in the
System
commit)@dotnet/area-system-reflection-metadata
@dotnet/area-system-reflection (for
System.Reflection.PortableExecutable
)@dotnet/area-system-runtime-compilerservices
@dotnet/area-system-runtime-compilerservices
@dotnet/area-system-runtime-intrinsics
@dotnet/area-system-security
@dotnet/area-system-text-json
@dotnet/area-system-text-regularexpressions
@dotnet/area-system-text-encoding
@kouvel for
System.Threading
@dotnet/area-system-threading-tasks
@Rick-Anderson for
System.Web
@RussKie for
System.Windows.Forms
@dotnet/area-system-runtime for
System