Improve formatting for Etc/GMT+X zones #7055
Conversation
robertbastian
requested review from
nekevss,
sffc and
zbraniecki
as code owners
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
robertbastian
requested review from
Manishearth
and removed request for
nekevss and
zbraniecki
components/time/src/zone/mod.rs
Outdated
| #[allow(clippy::identity_op, clippy::neg_multiply)] | ||
| let offset = match self.0.as_str().as_bytes() { | ||
| b"utc" | b"gmt" => Some(UtcOffset::zero()), | ||
| b"utce01" => { |
There was a problem hiding this comment.
thought: this is a lot of string search branches. the compiler might optimize it.
it may be possible to instead write this as a check for "utc", then a check for e or w, and then a numeric check?
There was a problem hiding this comment.
I'll let someone else have the satisfaction of optimising this should that ever be needed
|
We could set the offset and ID to unknown if the offsets don't match in |
|
this PR tests that |
3 participants
These should not format as "GMT+?", even if the offset is not set, and it shouldn't be possible to set a wrong offset.
The special-case code could also live in the specific format implementations in
icu_datetime, but there it would be duplicated. It could also live inGetField<UtcOffset>, but I like putting it inTimeZoneInfobecause then this is considered for equality ofTimeZoneInfo, and by extensionZonedDateTime.