-
Notifications
You must be signed in to change notification settings - Fork 239
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
DRIVERS-2870 Fix content of retryable-writes-test readme #1590
DRIVERS-2870 Fix content of retryable-writes-test readme #1590
Conversation
@@ -71,7 +71,7 @@ Drivers should also assert that command documents are properly constructed with | |||
on whether the write operation is supported. | |||
[Command Logging and Monitoring](../../command-logging-and-monitoring/command-logging-and-monitoring.rst) may be used to | |||
check for the presence of a `txnNumber` field in the command document. Note that command documents may always include an | |||
`lsid` field per the [Driver Session](../../sessions/driver-sessions.md) specification. | |||
`lsid` field per the [Driver Session](../../sessions/driver-sessions.rst) specification. |
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.
The session spec was converted in c6349d1. Shouldn't this remain as .md
?
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.
Fixed
`serverStatus` response. | ||
> [!NOTE] | ||
> Drivers that rely on `serverStatus` to determine the storage engine in use MAY skip this test for sharded clusters, | ||
> since `mongos` does not report this information in its `serverStatus` response. |
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.
These admonitions don't render within other elements, so I think this would be better written as a basic paragraph with no "Note" lead-in. Rendering issues aside, I think that makes more sense since the paragraph is providing direction for the test implementation by use of "MAY".
See this discussion and/or this test file for more info on rendering issues.
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.
They're left-aligned now.
Argh, there is something about this file that mdformat doesn't like, I'll keep trying...
|
The JSON failure is unrelated (and incorrect). The problem was that the admonition needed to be alone on its line... |
I am no longer an SDAM spec maintainer. I have removed myself from https://github.com/orgs/mongodb/teams/dbx-spec-owners-sdam, it wasn't updated when the maintainers changed. |
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.
LGTM with my last commit to add a missing step to the fifth prose test.
Thanks for the help! |
Please complete the following before merging:
clusters, and serverless).