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

Fix locale formatting for %c and %r #1058

Closed
wants to merge 10 commits into from
Closed

Conversation

scarf005
Copy link
Contributor

@pitdicker
Copy link
Collaborator

pitdicker commented May 12, 2023

The CI failures are not related to this PR. Edit: not all were 😄

src/format/locales.rs Outdated Show resolved Hide resolved
src/format/locales.rs Outdated Show resolved Hide resolved
src/format/strftime.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jtmoon79 jtmoon79 left a comment

Choose a reason for hiding this comment

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

More docstring recommendations (sorry for the churn, I noticed more on this second review of this PR).

src/format/locales.rs Show resolved Hide resolved
src/format/locales.rs Outdated Show resolved Hide resolved
src/format/locales.rs Outdated Show resolved Hide resolved
src/format/locales.rs Show resolved Hide resolved
src/format/locales.rs Outdated Show resolved Hide resolved
src/format/strftime.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jtmoon79 jtmoon79 left a comment

Choose a reason for hiding this comment

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

LGTM

@pitdicker
Copy link
Collaborator

Merged an approach that relies on changes in pure-rust-locales in #1165.
Thank you for your work and investigations here @scarf005!

@pitdicker pitdicker closed this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants