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

Explainer for API changes for calendar support #287

Merged
merged 11 commits into from
Dec 10, 2019

Conversation

sffc
Copy link
Collaborator

@sffc sffc commented Dec 7, 2019

I made a doc detailing my understanding of the state of the Temporal calendar design, following the 10+ hours of discussions this week at TC39.

@pipobscure @robpalme @apaprocki @spectranaut @caiolima

@codecov
Copy link

codecov bot commented Dec 7, 2019

Codecov Report

Merging #287 into main will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
- Coverage   78.06%   78.03%   -0.03%     
==========================================
  Files          17       17              
  Lines        3410     3410              
  Branches      338      338              
==========================================
- Hits         2662     2661       -1     
  Misses        733      733              
- Partials       15       16       +1
Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 91.62% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32d2ee5...527691b. Read the comment docs.

docs/calendar-draft.md Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
docs/calendar-draft.md Show resolved Hide resolved
docs/calendar-draft.md Outdated Show resolved Hide resolved
@sffc sffc dismissed gibson042’s stale review December 8, 2019 20:05

Comments resolved

@sffc
Copy link
Collaborator Author

sffc commented Dec 8, 2019

Thanks for all the comments! I created issues for all of the open questions. Let me know if I missed any. I plan to merge this PR soon.

@sffc
Copy link
Collaborator Author

sffc commented Dec 9, 2019

I have opened issues for outstanding issues. I'd like to keep remaining comments on this PR limited in scope to editorial issues. As soon as I have an LGTM I will merge the explainer into the repo, which should have all open questions labeled as such. Once the explainer is merged, we can contribute follow-on PRs as the open questions are addressed.

@pipobscure pipobscure merged commit b27be23 into tc39:main Dec 10, 2019
@littledan
Copy link
Member

Sorry for missing the Temporal meeting and this thread, but I'm missing something here: What's the motivation for a programmable calendar API, as opposed to just using an implementation-defined set of strings for calendars, like Intl does?

I'm curious, is it possible to get at the PartialIsoCalendar and mess with it?

@sffc
Copy link
Collaborator Author

sffc commented Dec 11, 2019

What's the motivation for a programmable calendar API, as opposed to just using an implementation-defined set of strings for calendars, like Intl does?

Follow-up: #298

I'm curious, is it possible to get at the PartialIsoCalendar and mess with it?

@pipobscure has an AI to make a polyfill for this from the Temporal monthly sync meeting this morning.

@littledan
Copy link
Member

Sorry, by "mess with it" I didn't mean try it out in a polyfill, but rather monkey-patch it and change the behavior of other Temporal.DateTime instances.

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.

7 participants