Skip to content
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-2789 Convert Unified Test Spec to Markdown #1503

Merged
merged 6 commits into from
Feb 8, 2024

Conversation

blink1073
Copy link
Member

Please complete the following before merging:

  • Update changelog.
  • Make sure there are generated JSON files from the YAML test files.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, sharded
    clusters, and serverless).

@blink1073 blink1073 requested review from a team as code owners February 6, 2024 12:50
@blink1073 blink1073 requested review from jmikola, durran, esha-bhargava, baileympearson, abr-egn and JamesKovacs and removed request for a team February 6, 2024 12:50
@blink1073 blink1073 requested review from a team and removed request for a team, JamesKovacs, durran, esha-bhargava, baileympearson and abr-egn February 6, 2024 12:50
@blink1073
Copy link
Member Author

I had to remove a dead link to https://emptysqua.re/blog/mongodb-testing-network-errors/.

Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of suggestions but nothing I'd need to follow up on.

source/unified-test-format/unified-test-format.md Outdated Show resolved Hide resolved

## Changelog

- 2024-02-06: Migrated from reStructuredText to Markdown.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I overlooked this in previous PRs but there's no way we can retain Definition Lists here, since it's non-standard syntax, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it doesn't render:

First Term
: This is the definition of the first term.

Second Term
: This is one definition of the second term.
: This is another definition of the second term.

Allow named KMS providers in `kmsProviders`. Note location of Client-Side
Encryption test credentials.

- 2024-01-03: Document server version requirements for `errorLabels` and\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://spec.commonmark.org/0.29/#example-631 suggests the trailing slash is used for a hard line break. I can see doing that for the "Schema version" highlights (although the bold text should already make them stand out), but why here and other entries with non-formatted text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why pandoc chose to do that

@jmikola
Copy link
Member

jmikola commented Feb 8, 2024

I had to remove a dead link to https://emptysqua.re/blog/mongodb-testing-network-errors/

@ajdavis: FYI, that article still comes up on your blog's search widget, but there's no icon for it and the page is a 404.

ajdavis added a commit to ajdavis/emptysquare-hugo that referenced this pull request Feb 8, 2024
@ajdavis
Copy link
Member

ajdavis commented Feb 8, 2024

Oops, I restored that article to my site now. I (wrongly?) thought it was obsolete and deleted it a while ago.

@blink1073 blink1073 merged commit f017041 into mongodb:master Feb 8, 2024
4 checks passed
@blink1073 blink1073 deleted the DRIVERS-2789-unified-test branch February 8, 2024 19:54
@jmikola
Copy link
Member

jmikola commented Feb 12, 2024

@blink1073: I just realized that various internal link targets in the converted Unified Test Format spec were changed to (<>). I assume that was unintentional, but is it a common problem with the RST-to-MD conversions?

@blink1073
Copy link
Member Author

The original doc had [observeEvents](<>), the conversion must not know what to do with that syntax.

@ajdavis
Copy link
Member

ajdavis commented Feb 15, 2024

I'm unsubscribing, if you need me for anything LMK.

@jmikola
Copy link
Member

jmikola commented Feb 17, 2024

The original doc had observeEvents, the conversion must not know what to do with that syntax.

This seems related to original RST links to internal page anchors.

`storeEventsAsEntities <entity_client_storeEventsAsEntities_>`_

...was changed to:

[storeEventsAsEntities](<>)

Assuming this wasn't the only spec affected, can you check for this in any outstanding PRs? For things that were already merged, I suppose it can be handled in a follow-up ticket within the epic. The broken links should definitely not remain as-is.

eramongodb added a commit to eramongodb/specifications that referenced this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants