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

Med: libcrmcommon/iso8601: fix train of thinkos regarding time offsets #2737

Closed
wants to merge 1 commit into from

Conversation

waltdisgrace
Copy link
Contributor

@waltdisgrace waltdisgrace commented Jun 16, 2022

closes T188

Copy link
Contributor

@kgaillot kgaillot left a comment

Choose a reason for hiding this comment

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

I'm thinking the code base has changed enough that it's now misleading to say that poki authored this, so you should commit your changes fresh and just mention in the comment that it's a forward-port of work he originally did

Rebasing this on current main should clarify it a bit

Due to a bug, timezone-containing format strings were were being
parsed incorrectly in the following ways:

- White space was optional when it should've been forbidden, e.g.
  +02 00 got parsed as if it were +0200.

- White space was required when it should've been optional, e.g.
  no timezone in "2019-04-25T21:42:54+0200" got parsed, whereas
  a timezone in "2019-04-25T21:42:54 +0200" got parsed.

- Timezones beginning with a '+' were parsed incorrectly, e.g.
  +0200 got parsed as +0020 due to '+' "consuming" a position.

In the future, we should use a library such as glib's GDateTime
(https://developer.gnome.org/glib/stable/glib-GDateTime.html#g-date-time-new-from-iso8601)
and update any necessary schema pocedures.

This PR was initially authored by jnpkrn and heavily edited by me to
reflect the current code base.

closes T188
@waltdisgrace waltdisgrace marked this pull request as ready for review July 12, 2023 15:10
@kgaillot
Copy link
Contributor

kgaillot commented Dec 4, 2024

Doh, I completely overlooked the last changes to this PR :-/ Sorry for letting this linger so long ...

Closing this since the code base has changed significantly, and it will need a rebase and re-review, so a new PR would be better.

@kgaillot kgaillot closed this Dec 4, 2024
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