minor: fix documentation of new features #823

isabelatkinson · 2023-02-03T15:47:12Z

As far as I can tell there's not a great way to test what this is going to look like on docs.rs short of doing a release, but the FLE docs should show up in the same way that the sync docs currently do.

isabelatkinson · 2023-02-03T16:20:20Z

src/lib.rs

+//! | `in-use-encryption-unstable` | Enable support for client-side field level encryption and queryable encryption. This API is unstable and may be subject to breaking changes in minor releases.          | no      |
+//! | `tracing-unstable`           | Enable support for emitting [`tracing`](https://docs.rs/tracing/latest/tracing/) events. This API is unstable and may be subject to breaking changes in minor releases. | no      |


These are the two new additions to the table; I realized they were missing. I also removed the "Extra dependencies" column as a lot of the information was outdated/incorrect. If we think this information is valuable then I can go through and fix it, but I'm not sure how useful it is given that users can just look at our Cargo.toml. There's also a risk that it will become out-of-date again in the future if we add/change dependencies and forget to update here. Thoughts?

Agreed this doesn't need to be in the table.

isabelatkinson · 2023-02-03T16:21:13Z

src/lib.rs

@@ -343,7 +345,8 @@ mod test;
 #[cfg(feature = "tracing-unstable")]
 mod trace;


I don't think there's anything here that needs to be exported publicly/documented but let me know if that's wrong, @kmahar.

the only thing explicitly in the API around tracing is the tracing_max_document_length_bytes option in ClientOptions. that already has the docsrs annotations and appears to show up correctly in the docs: https://docs.rs/mongodb/2.4.0-beta.1/mongodb/options/struct.ClientOptions.html#structfield.tracing_max_document_length_bytes

abr-egn · 2023-02-03T16:35:25Z

src/lib.rs

+//! | `in-use-encryption-unstable` | Enable support for client-side field level encryption and queryable encryption. This API is unstable and may be subject to breaking changes in minor releases.          | no      |
+//! | `tracing-unstable`           | Enable support for emitting [`tracing`](https://docs.rs/tracing/latest/tracing/) events. This API is unstable and may be subject to breaking changes in minor releases. | no      |


Agreed this doesn't need to be in the table.

abr-egn · 2023-02-03T16:39:43Z

src/lib.rs

@@ -343,7 +345,8 @@ mod test;
 #[cfg(feature = "tracing-unstable")]
 mod trace;

-#[cfg(feature = "in-use-encryption-unstable")]
+#[cfg(any(feature = "in-use-encryption-unstable", docsrs))]
+#[cfg_attr(docsrs, doc(cfg(any(feature = "sync", feature = "tokio-sync"))))]


I think the second line should be

#[cfg_attr(docsrs, doc(cfg(feature = "in-use-encryption-unstable")))]

(and below).

whoops, bad copy/paste - fixed

kmahar

LGTM mod Abraham's comment.

kmahar · 2023-02-03T17:24:10Z

Cargo.toml

 # The In Use Encryption API is unstable and may have backwards-incompatible changes in minor version updates.
 in-use-encryption-unstable = ["mongocrypt", "rayon", "num_cpus"]

-# DO NOT USE; see https://jira.mongodb.org/browse/RUST-580 for the status of tracing/logging support in the Rust driver.


thanks for removing this! I forgot :)

kmahar · 2023-02-03T17:24:35Z

src/lib.rs

+//! | `in-use-encryption-unstable` | Enable support for client-side field level encryption and queryable encryption. This API is unstable and may be subject to breaking changes in minor releases.          | no      |
+//! | `tracing-unstable`           | Enable support for emitting [`tracing`](https://docs.rs/tracing/latest/tracing/) events. This API is unstable and may be subject to breaking changes in minor releases. | no      |


kmahar · 2023-02-03T17:29:38Z

src/lib.rs

@@ -343,7 +345,8 @@ mod test;
 #[cfg(feature = "tracing-unstable")]
 mod trace;


the only thing explicitly in the API around tracing is the tracing_max_document_length_bytes option in ClientOptions. that already has the docsrs annotations and appears to show up correctly in the docs: https://docs.rs/mongodb/2.4.0-beta.1/mongodb/options/struct.ClientOptions.html#structfield.tracing_max_document_length_bytes

abr-egn

LGTM, thanks!

isabelatkinson · 2023-02-08T17:20:37Z

The new changes update our docs.rs build approach to use feature flags rather than the #[cfg(docsrs)] attribute. Currently, anything behind a feature flag that we want to document publicly has annotations to the effect of:

#[cfg(any(feature = "feature", docsrs))]
#[cfg_attr(docsrs, doc(cfg(feature = "feature")))]

The first line includes the code in our docs build and the second line adds a note in the docs that the item is gated behind a feature flag. We also have the following in our Cargo.toml:

rustdoc-args = ["--cfg", "docsrs"]

This approach was necessary in part because the doc(cfg(...)) annotation is only available on nightly, so we needed to gate its usage behind docsrs. However, a new crate-wide doc_auto_cfg annotation was recently added which will automatically add feature-flag notes in the docs. Because this cfg_attr is no longer needed, we don't really need to use docsrs throughout the code anymore. We do still need it to gate the usage of doc_auto_cfg in lib.rs, as that is also only available on nightly.

The new approach is to build our documentation with all of the non-default-conflicting features (i.e. everything but async-std-runtime and sync) specified rather than adding docsrs inline to anything public behind a feature flag. We can get away with this because we don't have any public API associated with async-std-runtime and all of the API associated with sync also builds with the tokio-sync feature. (doc_auto_cfg will still make a note that our sync API is available with the sync feature flag.) I find this new approach to be more maintainable because we no longer need to remember to add a docsrs annotation when we introduce new feature-flag-gated API. However, if we do add any API gated behind those conflicting features, we can fall back to the docsrs annotation in those cases.

isabelatkinson · 2023-02-08T15:26:13Z

Cargo.toml

+    "aws-auth",
+    "tracing-unstable",
+    "in-use-encryption-unstable"
+]


The main drawback of this new approach is that we need to keep a manual list of all of our features and be sure to append here as new ones are added. I do think this is still a slight improvement to the current situation, though, as this is an easier change than adding #[cfg(docsrs)] to any public API behind a feature flag.

This becomes much easier if we choose to remove our async-std-related features in the future, as we can just specify to use all features 🙂

Yup, agreed, definitely prefer this way.

isabelatkinson · 2023-02-08T15:27:25Z

src/lib.rs

-// `missing_crate_level_docs` was renamed with a `rustdoc::` prefix in rustc 1.55, but isn't
-// supported in the MSRV.
-// TODO: remove the wrapping cfg_attr if/when the MSRV is 1.55+.
-#![cfg_attr(docsrs, warn(rustdoc::missing_crate_level_docs))]


This is slightly unrelated but I noticed that we can remove this now that our MSRV is higher than 1.55.

abr-egn

This is much better than how it was set up before - thank you!

abr-egn · 2023-02-08T20:33:49Z

Cargo.toml

+    "aws-auth",
+    "tracing-unstable",
+    "in-use-encryption-unstable"
+]


Yup, agreed, definitely prefer this way.

isabelatkinson added 2 commits February 3, 2023 10:46

minor: fix FLE documentation

d3baf04

fix table

a8bd8c0

isabelatkinson commented Feb 3, 2023

View reviewed changes

isabelatkinson changed the title ~~minor: fix FLE documentation~~ minor: fix documentation of new features Feb 3, 2023

isabelatkinson requested review from abr-egn and kmahar February 3, 2023 16:22

abr-egn reviewed Feb 3, 2023

View reviewed changes

kmahar approved these changes Feb 3, 2023

View reviewed changes

fix feature

98b995a

isabelatkinson requested a review from abr-egn February 3, 2023 20:10

abr-egn approved these changes Feb 3, 2023

View reviewed changes

isabelatkinson added 2 commits February 8, 2023 10:19

use doc_auto_cfg

1ac4cfa

fix feature combos

39384c3

isabelatkinson commented Feb 8, 2023

View reviewed changes

isabelatkinson requested a review from abr-egn February 8, 2023 17:27

abr-egn approved these changes Feb 8, 2023

View reviewed changes

Cargo.toml

"aws-auth",

"tracing-unstable",

"in-use-encryption-unstable"

]

Copy link

Contributor

abr-egn Feb 8, 2023

Choose a reason for hiding this comment

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

Yup, agreed, definitely prefer this way.

isabelatkinson merged commit a5117fd into mongodb:main Feb 8, 2023

isabelatkinson added a commit that referenced this pull request Feb 9, 2023

minor: fix documentation of new features (#823)

76ffb2e

isabelatkinson deleted the fix-fle-docs branch March 6, 2023 18:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minor: fix documentation of new features #823

minor: fix documentation of new features #823

isabelatkinson commented Feb 3, 2023 •

edited

Loading

isabelatkinson Feb 3, 2023

abr-egn Feb 3, 2023

kmahar Feb 3, 2023

isabelatkinson Feb 3, 2023

kmahar Feb 3, 2023

abr-egn Feb 3, 2023

abr-egn Feb 3, 2023

isabelatkinson Feb 3, 2023

kmahar left a comment

kmahar Feb 3, 2023

kmahar Feb 3, 2023

kmahar Feb 3, 2023

abr-egn left a comment

isabelatkinson commented Feb 8, 2023 •

edited

Loading

isabelatkinson Feb 8, 2023

abr-egn Feb 8, 2023

isabelatkinson Feb 8, 2023

abr-egn left a comment

abr-egn Feb 8, 2023

		//! \| `in-use-encryption-unstable` \| Enable support for client-side field level encryption and queryable encryption. This API is unstable and may be subject to breaking changes in minor releases. \| no \|
		//! \| `tracing-unstable` \| Enable support for emitting [`tracing`](https://docs.rs/tracing/latest/tracing/) events. This API is unstable and may be subject to breaking changes in minor releases. \| no \|

		@@ -343,7 +345,8 @@ mod test;
		#[cfg(feature = "tracing-unstable")]
		mod trace;

minor: fix documentation of new features #823

minor: fix documentation of new features #823

Conversation

isabelatkinson commented Feb 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmahar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abr-egn left a comment

Choose a reason for hiding this comment

isabelatkinson commented Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abr-egn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelatkinson commented Feb 3, 2023 •

edited

Loading

isabelatkinson commented Feb 8, 2023 •

edited

Loading