DRIVERS-3667 Drivers specification standards

[NoahStapp]

Please complete the following before merging:

• Is the relevant DRIVERS ticket in the PR title?

• [ ] Update changelog.
• [ ] Test changes in at least one language driver.
• [ ] Test these changes against all server versions and topologies (including standalone, replica set, and sharded
  clusters).

[matthewdale]

This seems overly prescriptive. We should suggest but not require all sections. We should require the order is the same, and require a subset of sections.

For example:

Specifications must use the following sections, in this order, when present. Abstract, META, Specification, and Changelog are required.

[NoahStapp]

Fair enough, no use in having empty sections just to fit a structure.

[matthewdale]

Do we intend to follow RFC 2119 in the contributing guidelines? If not, it's worth calling out that this document does not follow RFC 2119, since drivers devs are used to reading spec documents with "MUST".

[NoahStapp]

You mean that we should explicitly call out that RFC 2119 keywords are exempt from this rule?

[NoahStapp]

Re-reading I see what you mean. I would prefer to not use RFC 2119 here since all of these rules are effectively MUST.

[dariakp]

I don't think that's true: re all rules being effectively "MUST" - there is a mix of requirements and "soft" recommendations, e.g. "consider", "prefer".

Moreover, the current list of bullet points mixes directives with statements. Using "MUST" in a lot of places might feel like too loud, but I think it would make this easier to read and follow (i.e., harder to misinterpret intent or argue down the line about whether something is a soft guideline vs a hard requirement). It would also make the standards page more cohesive with the specs themselves.

[NoahStapp]

Fair enough. I read all of them as having an implicit "MUST" (e.x. "MUST consider..."), but that's less explicit than actually having the words there.

[jyemin]

Consider expanding this, something like:

• Add tests to a tests directory
  ** Add prose tests to a README.md file in the tests directory
  ** Add unified tests to a unified directory in the tests directory

[prestonvasquez]

[question] What is the historical reasoning for Javascript / Typescript pseudocode? These seem to be the widely used language for algorithms in the specifications.

Do we anticipate that this rule is forward-only, or does it mean we plan to convert the existing algorithm blocks? If we plan to convert, we should create a ticket and link it here as a future effort.

Edit: python syntax is the most common for algorithms if you don't count BSON / mongosh.

[NoahStapp]

I assume because they have minimal syntax overhead and are widely readable regardless of language knowledge, but @jyemin might have a more historical answer.

We do plan to convert the existing blocks. I'll create a ticket to track that work. I'm not sure we need to link it in this document since it doesn't relate for future specification work.

[prestonvasquez]

I assume because they have minimal syntax overhead and are widely readable regardless of language knowledge

What does python syntax improve on this?

I'm not sure we need to link it in this document since it doesn't relate for future specification work.

If we plan to convert the existing blocks, that conversion is a consequence of this rule so it does relate to this document. Unless we decided to make this forward-only. Either works for me, but I'd like the document to say which, since future PRs against existing non-python syntax will have to guess. e.g. "if the spec PR modifies a few lines of an existing JS algorithm, is the author expected to convert the entire block?"

There is precedent to linking driver tickets for future work in specifications, https://github.com/search?q=repo%3Amongodb%2Fspecifications+DRIVERS-&type=code

[NoahStapp]

What does python syntax improve on this?

I don't understand the question. Python is as close as languages get to being English text with some syntax, making it very easy to read psuedocode.

If we plan to convert the existing blocks, that conversion is a consequence of this rule so it does relate to this document. Unless we decided to make this forward-only. Either works for me, but I'd like the document to say which, since future PRs against existing non-python syntax will have to guess. e.g. "if the spec PR modifiees a few lines of an existing JS algorithm, is the author expected to convert the entire block?"

There is precedence to linking driver tickets for future work in specifications, https://github.com/search?q=repo%3Amongodb%2Fspecifications+DRIVERS-&type=code

We'll have a single PR to update all of the existing psuedocode to these standards so authors won't have to determine that. I'd prefer to keep transient references like tickets out of a standards document.

[prestonvasquez]

Suggest deferring the pseudocode decision to the DRIVERS ticket so that the style guide and existing specs change together.

[dariakp]

Which category do the API interface definitions fall under?

We could potentially resolve the tension between the timeline of the code style standardization conversion of the specifications with the strict language of these standards by saying something like "all newly added pseudocode MUST ..." and then update the standards wording to remove the "newly" qualifier after we've standardized existing code. Otherwise I think we should just omit the current requirement until after we've converted the code and instead capture the addition of this standards guideline to the AC of the ticket for updating the pseudocode, so that they could be done together.

I do agree with Noah though on not adding explicit ticket references to the standards doc itself because a promised standard isn't yet an accepted standard, and we want people to follow what's formally agreed on, not what might be agreed on.

[NoahStapp]

Good catch. API interfaces would be (and already are) better defined in Typescript, I'll add that clarification.

I'm not especially concerned that existing specs won't immediately align to these standards. Almost all of our existing pseudocode already uses Python or Typescript syntax, and fixing the exceptions is easy.

[sanych-sun]

Should we mention that each change MUST have related Drivers ticket?

[sanych-sun]

Should we mention md format is required for all specs?

[sanych-sun]

Should we add link to the CONTRIBUTING.md from the README.md, and probably move some content from README into this document? For example: "Writing Documents", "Prose Test Numbering", "Automated Test Best Practices" sections sounds like they are belong to the CONTRIBUTING.md too.

[jyemin]

A few other things to consider before merging:

1. RFT 8174 clarifies usage of all-caps MUST, SHOULD etc. Consider referencing it
2. The AGENTS.md file has several recommendations that more properly belong here, e.g. 120-char GFM line width, never hand-edit generated JSON/YAML is source of truth, always use the lowest schema version that works, prose tests use relative 1. numbering and new tests append at the end (drivers reference by number), test isolation (omit irrelevant fields), don't modify existing tests unless they test wrong behavior, PR requirements (DRIVERS ticket, driver implementation PR links in description).
3. Mention the automated enforcement layer in .pre-commit-config.yaml and .github/workflows/lint.yml

[NoahStapp]

A few other things to consider before merging:

1. [RFT 8174](https://www.rfc-editor.org/info/rfc8174/) clarifies usage of all-caps MUST, SHOULD etc.  Consider referencing it

2. The AGENTS.md file has several recommendations that more properly belong here, e.g. 120-char GFM line width, never hand-edit generated JSON/YAML is source of truth, always use the lowest schema version that works, prose tests use relative 1. numbering and new tests append at the end (drivers reference by number), test isolation (omit irrelevant fields), don't modify existing tests unless they test wrong behavior, PR requirements (DRIVERS ticket, driver implementation PR links in description).

3. Mention the automated enforcement layer in `.pre-commit-config.yaml` and `.github/workflows/lint.yml`

I've moved all content that fits from README.md and AGENTS.md into CONTRIBUTING.md.

[prestonvasquez]

[question] Is this intended for all tests or just unified tests? If prose tests are included, then we should define some mechanism to make it not conflict with L63. E.g. "Since drivers reference prose tests by number, once a prose test has been removed by all drivers then its contents MAY be replaced with a note such as Removed."

[NoahStapp]

Good catch. I would honestly prefer us to just remove unused prose tests as well and accept the one-time re-numbering cost.

[dariakp]

Renumbering tests would make for an auditing nightmare for individual drivers. IIRC the previous guidance was explicit about "deleting" tests by removing the test prose but leaving the numbering in place with a "REMOVED" marker. I much prefer that approach.

[NoahStapp]

Is it that big of a deal to renumber the affected prose tests? We don't delete prose tests often enough for it to be a constant headache, and it's confusing reading tests where the numbering jumps around. I'd prefer the specifications and their tests to accurately reflect the latest state rather than contain historical artifacts wherever possible.

[prestonvasquez]

[question] What do you think about adding labels to the test standards to make them referenceable, similar to prose tests?

- **T1.** Tests MUST cover every behavior required in the specification.

[NoahStapp]

I like it, but we should add labels to each section if we're going to do so. Otherwise we should just leave it as-is.

[prestonvasquez]

Suggest we note somewhere that specifications are self-defining: "A specification is a document under source/ whose header block includes a Status line". That way we can still have markdown files (or whatever) in source without it falling under the requirements of this standard.

[NoahStapp]

Good point, I'll add a self-defining statement. Honestly I think we should remove the status lines everywhere, they aren't useful.

[NoahStapp]

Actually I don't think we need this: it's clear if a document is a specification or not by definition and therefore if these rules apply. I don't believe this document needs to define a specification, only the standards for documents that are known to be specifications.

[prestonvasquez]

[blocking] These requirements can't be satisfied by editorial PRs (such as this one). Suggest that we state that somewhere: e.g., "PRs that do not change driver behavior or tests, such as editorial changes, are exempt from the driver requirements above."

Or if this change is accepted: https://github.com/mongodb/specifications/pull/1992/changes#r4200664929, something like: "PRs that do not change driver behavior or tests, such as editorial changes, are exempt from the driver requirements PR2 and PR3."

[jyemin]

Let's add something like this:

Before writing or reviewing any specification change, read [CONTRIBUTING.md](./CONTRIBUTING.md). It defines the standards for prose, pseudocode, tests, change logs, and PRs.

Otherwise an agent may ignore CONTRIBUTING.md, which is sub-optimal.

[jyemin]

LGTM!

[dariakp]

Which category do the API interface definitions fall under?

We could potentially resolve the tension between the timeline of the code style standardization conversion of the specifications with the strict language of these standards by saying something like "all newly added pseudocode MUST ..." and then update the standards wording to remove the "newly" qualifier after we've standardized existing code. Otherwise I think we should just omit the current requirement until after we've converted the code and instead capture the addition of this standards guideline to the AC of the ticket for updating the pseudocode, so that they could be done together.

I do agree with Noah though on not adding explicit ticket references to the standards doc itself because a promised standard isn't yet an accepted standard, and we want people to follow what's formally agreed on, not what might be agreed on.

[dariakp]

Renumbering tests would make for an auditing nightmare for individual drivers. IIRC the previous guidance was explicit about "deleting" tests by removing the test prose but leaving the numbering in place with a "REMOVED" marker. I much prefer that approach.