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

Encode: Follow RFC3339 spec for LocalTime #632

Merged

Conversation

jidicula
Copy link
Contributor

@jidicula jidicula commented Oct 17, 2021

Issue: #613

Fixes 2/3 failing subcases for go test -tags testsuite -run TestTOMLTest_Valid_Datetime_Local:

  • local
  • space

Note that the remaining failing case is addressed by #626.

The fix for this was quite straightforward: RFC3339 specifies a T between date and time, not a as the String() method on LocalDateTime was formatting. See go/time constants for verification: https://pkg.go.dev/time#pkg-constants .

@jidicula
Copy link
Contributor Author

jidicula commented Oct 17, 2021

@pelletier I took the liberty of removing the skip statement for the failing testgen case, but since one of the subcases is covered in #626 and not here, the CI tests will fail. If you inspect the workflow run log, you'll notice that the only failing case is milli (you can see local and space pass if you run go test -v -tags testsuite -run TestTOMLTest_Valid_Datetime_Local). I could:

Let me know what I should do 🙂

@pelletier
Copy link
Owner

@jidicula do you mind rebasing against v2? Just merged #626.

@jidicula jidicula force-pushed the ji/issue-613-valid-datetime-localtime branch from 398673e to 8b2dd6e Compare October 18, 2021 01:00
@jidicula
Copy link
Contributor Author

@jidicula do you mind rebasing against v2? Just merged #626.

Done!

@pelletier
Copy link
Owner

Looks like the tests for LocalTime need to be adjust to use T as a separator.

@pelletier pelletier merged commit df4bb06 into pelletier:v2 Oct 18, 2021
@pelletier
Copy link
Owner

Thank you for the patch!

@jidicula jidicula deleted the ji/issue-613-valid-datetime-localtime branch October 18, 2021 14:02
@pelletier pelletier changed the title Follow RFC3339 spec for datetime format Encode: Follow RFC3339 spec for LocalTime Oct 28, 2021
@pelletier pelletier added the bug Issues describing a bug in go-toml. label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml. hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants