-
Notifications
You must be signed in to change notification settings - Fork 9.1k
feat: add date-time-local and time-local. #4760
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
base: gh-pages
Are you sure you want to change the base?
Conversation
@baywet @mikekistler thoughts? |
No objection from me. |
The `{{page.slug}}` format represents a time without timezone for [TOML Local Time](https://toml.io/en/v1.0.0#local-time) compatibility. | ||
{% endcapture %} | ||
|
||
{% include format-entry.md summary=summary %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we not also including date local?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Time Zone is tied to time, date-local
was not suggested.
registries/_format/time-local.md
Outdated
--- | ||
owner: ya7010 | ||
issue: 4759 | ||
description: time without timezone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mention it's based on RFC 3339?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Time zone is required by RFC 3339.
This PR proposes a format without time zone, which is not defined in the RFC but defined in TOML.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, what I meant was maybe we should update the text to say something like "RFC3339 time without the timezone component" to clarify the base for it is the RFC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misunderstood.
That makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed: 3301746
@@ -0,0 +1,15 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the name, I've seen programming languages call this a time only. I think it'd be interesting to see whether multiple languages use the same name (time only or otherwise) and check whether it's worth using a different name for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your point is correct.
On the other hand, the already existing time
and date-time
format follow RFC3339, and thus enforce the time zone.
So, I think the proposed format with *-local
is the right direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that given that time
is already a format, time-local
seems the most clear. Unless you had a specific alternative in mind @baywet ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time-only was the alternative I was thinking about here. But I'm probably coming with too much of a dotnet angle
java calls it local time
go uses the same struct for everything
I guess I'm good with time-local, resolving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(well, I can't resolve, please go ahead and resolve this comment)
Fix: #4759