Address JsonElement doc and test feedback#43832
Conversation
| ex = e; | ||
| } | ||
|
|
||
| Assert.Equal(0, reader.BytesConsumed); |
| ex = null; | ||
| } | ||
| catch (JsonException) { } | ||
| catch (Exception e) |
There was a problem hiding this comment.
Should this be:
| catch (Exception e) | |
| catch (JsonException e) |
There was a problem hiding this comment.
It could be, but I followed the pattern in JsonDocumentTests, which I believe wants to fail with a nice Assert instead of a unhandled exception.
| /// </remarks> | ||
| /// <exception cref="ArgumentException"> | ||
| /// <paramref name="reader"/> is using unsupported options. | ||
| /// <paramref name="reader"/> contains unsupported options. |
There was a problem hiding this comment.
Is this where we landed on the doc sentence?
From reading #43601 (comment), I thought we'd keep "is using unsupported options" across the board instead (and update dotnet-api-docs accordingly).
| /// <paramref name="reader"/> contains unsupported options. | |
| /// <paramref name="reader"/> is using unsupported options. |
There was a problem hiding this comment.
Since the docs have been reviewed after the original comments in code were ported, I assumed docs win.
However, it looks a little messy:
- The original code used "contains" and is still using that
- When originally ported to dotnet-api-docs it used "contains"
- Another pass was made to dotnet-api-doc changing from "is using" to "contains" although history doesn't show the "is using" change...
There was a problem hiding this comment.
I reverted the "contains" changes.
However, there are also similar "contains unsupported options" elsewhere for JsonDocumentOptions; I don't have a preference here.
There was a problem hiding this comment.
Is there an issue tracking the docs update, in dotnet-api-docs, to make the wording consistent (is using unsupported options)?
It's nice to have the XML comments in source match what's in the official docs/IntelliSense, but not absolutely necessary. The priority is getting the donet-api-docs to be accurate and consistent because that is the source of truth, which customers/end users see.
Address feedback from #43601