Skip to content

Conversation

@paulcdejean
Copy link

No description provided.

@paulcdejean paulcdejean marked this pull request as draft October 25, 2025 20:19
@paulcdejean
Copy link
Author

Fixes #2425

@paulcdejean
Copy link
Author

There's currently TODOs all over the place here. I did try and dig through the code and see what the feature flags were actually doing but I'm not going to have the same depth of understanding as a maintainer.

Happy to update this based on feedback, however I'm not going to be able to complete the existing TODOs on my own without feedback or assistance.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Thanks for getting this started!

Copy link
Collaborator

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

@paulcdejean paulcdejean marked this pull request as ready for review October 29, 2025 02:16
@paulcdejean
Copy link
Author

Ready for re review! Thanks for all the helpful feedback.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

A few more nits. I'll make another pass after this is merged.

Please make sure to squash all of your commits and fix the CI issues.

@cathaysia
Copy link

Do you know this crate: https://docs.rs/document-features/latest/document_features/

it's very useful for this case.

@djc
Copy link
Member

djc commented Oct 30, 2025

(If you rebase on main, the WASM failure in CI should go away.)

@paulcdejean
Copy link
Author

I rebased. Should I also squash? Or will do you the squashing?

@paulcdejean
Copy link
Author

I saw you said to squash so I squashed.

//!
//! - `runtime-tokio`: Enable integration with the tokio async runtime.
//! - `runtime-smol`: Enable integration with the smol runtime.
//! - `smol`: Also enable integration with the smol runtime.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between runtime-smol and smol? Should be explained here.

Copy link
Author

Choose a reason for hiding this comment

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

I have no idea. @djc do you have any idea? Looking at the code I'm not sure how to better document this.

@paulcdejean
Copy link
Author

@Frando I reworked the crypto section based on your feedback and based on digging into the code more.

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.

5 participants