Skip to content

Move string types to separate module#329

Merged
cpu merged 2 commits into
mainfrom
string_types_module
Apr 18, 2025
Merged

Move string types to separate module#329
cpu merged 2 commits into
mainfrom
string_types_module

Conversation

@est31

@est31 est31 commented Apr 13, 2025

Copy link
Copy Markdown
Member

Looking at the rustdoc, it's a bit weird to see the string types mixed up with certificate related stuff.

The rename was done because it follows the way std names its modules.

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

@est31 est31 force-pushed the string_types_module branch from b7e3213 to 9e54673 Compare April 14, 2025 00:24

@cpu cpu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! I like it.

I think there are a couple CI failures to sort out, one of which is a semver compat check issue I left a comment about.

Comment thread rcgen/src/lib.rs
@cpu cpu force-pushed the string_types_module branch from 85e90a9 to 64279e3 Compare April 18, 2025 15:46
@cpu

cpu commented Apr 18, 2025

Copy link
Copy Markdown
Member

cpu force-pushed the string_types_module branch from 85e90a9 to 64279e3

I fixed a build failure in the version bump commit (rustls-cert-gen needed its path import version updated), as well as a doc test failure from the module refactoring (changing use crate::x to use crate::strings::x throughout).

It seemed easier to fix it & push than to do another back-and-forth round. Hope you don't mind.

@cpu cpu enabled auto-merge April 18, 2025 15:48
@cpu cpu added this pull request to the merge queue Apr 18, 2025
Merged via the queue into main with commit cf4eb2d Apr 18, 2025
@cpu cpu deleted the string_types_module branch April 18, 2025 16:23
@djc djc mentioned this pull request Jun 25, 2025
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.

3 participants