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

Dates are written with spaces #87

Closed
ArthurClemens opened this issue Jul 31, 2022 · 8 comments · Fixed by #90
Closed

Dates are written with spaces #87

ArthurClemens opened this issue Jul 31, 2022 · 8 comments · Fixed by #90
Labels
enhancement New feature or request

Comments

@ArthurClemens
Copy link

Given a resource struct:

[
  %{
    inserted_at: ~U[2022-07-31 14:48:48.000000Z]
  }
]

The YAML output is:

- inserted_at:
    2022-07-31 14:48:48.000000000 Z

When reading this data back in the application, this is not directly usable to create dates with.

For comparison, Jason encodes this as:

[
  {
    "inserted_at": "2022-07-31T14:48:48.000000Z"
  }
]

which can directly be read using DateTime.from_iso8601()

@jlgeering
Copy link
Collaborator

https://yaml.org/type/timestamp.html

current implementation returns a valid timestamp but not in the canonical format => probably better to switch

would require a major version bump => @mruoss what do you think?

@mruoss
Copy link
Collaborator

mruoss commented Aug 2, 2022

Yes thanks. Was about to check on yaml.org. Maybe we should have an issue template whwere it asks to point to official doc.

So yes, according to doc, the current version is valid YAML but not canonical. Should it be? And yes, would be a major version. And not adapted to the 1.x version.

@mruoss mruoss closed this as completed Aug 2, 2022
@mruoss mruoss reopened this Aug 2, 2022
@mruoss
Copy link
Collaborator

mruoss commented Aug 2, 2022

Ooops, wrong button, sorry.

@jlgeering
Copy link
Collaborator

@mruoss for now, I'm would opt to not make this configurable (wait for a concrete need to add some form of DateTime format option / param)

@mruoss
Copy link
Collaborator

mruoss commented Aug 2, 2022

@jlgeering Make what configuratble? The output format? I would not make that configurable...

@jlgeering
Copy link
Collaborator

@joerichsen FYI this change might impact you

@joerichsen
Copy link
Contributor

I added the PR with DateTime support in order to read YAML stored in strings in our database, where data is stored by a Ruby on Rails application.

So I just added the same format as Ruby stored it in in the PR.

But I just checked and it seems like Ruby is able to read the canonical format just fine as well, so I think that using the canonical format instead makes more sense 👍

@mruoss
Copy link
Collaborator

mruoss commented Aug 5, 2022

Perfect. Thanks for your heads up @joerichsen.

mruoss pushed a commit that referenced this issue Aug 5, 2022
encode timestamp with canonical format (iso8601) see #87
@mruoss mruoss closed this as completed in #90 Aug 5, 2022
@mruoss mruoss added the enhancement New feature or request label Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants