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

Jump to bottom

Bikeshedding the name of the inZone method #235

Closed
littledan opened this issue Oct 30, 2019 · 4 comments · Fixed by #248
Closed

Bikeshedding the name of the inZone method #235

littledan opened this issue Oct 30, 2019 · 4 comments · Fixed by #248
Assignees
@ryzokuken
Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
Finalize spec text Finalize polyfill
Milestone
November Meeting

Comments

@littledan
Copy link
Member

littledan commented Oct 30, 2019 •
edited

Now that we have Temporal.TimeZone, should we call this method inTimeZone?
@littledan littledan changed the title Bikeshedding the name of the withZone method Bikeshedding the name of the inZone method Oct 30, 2019
@ryzokuken
Copy link
Member

In a quick albeit shallow search of the codebase (unfortunately the search functionality in VSCode doesn't allow negative lookahead), I failed to come up with the usage of "zone" anywhere apart from timezones, so +1 to this.

@maggiepint
Copy link
Member

No opposition, especially if it creates API parity.

@ryzokuken
Copy link
Member

@pipobscure do you have any contradicting opinions on this by any chance? Assigning this to myself, will make a PR for it today.

@ryzokuken ryzokuken self-assigned this Oct 31, 2019
@ryzokuken ryzokuken added non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved labels Oct 31, 2019
@ryzokuken ryzokuken added this to To do in Finalize spec text via automation Oct 31, 2019
@ryzokuken ryzokuken added this to To do in Finalize polyfill via automation Oct 31, 2019
@ryzokuken ryzokuken added this to the November Meeting milestone Oct 31, 2019
@pipobscure
Copy link
Collaborator

The only thought I have is that it’s long. Can we shorten it to just in 😄

so dateTime.in(‘America/New_York’)

(yes let’s go with inTimeZone)

ryzokuken added a commit to ryzokuken/proposal-temporal that referenced this issue Nov 1, 2019
@ryzokuken
[api] rename inZone to inTimeZone
569e1c4 
Rename `Absolute.prototype.inZone` and `DateTime.prototype.inZone` to
`Absolute.prototype.inTimeZone` and `DateTime.prototype.inTimeZone`
respectively, since Temporal.TimeZone now exists.

Fixes: tc39#235
@ryzokuken ryzokuken mentioned this issue Nov 1, 2019
Merged
Finalize spec text automation moved this from To do to Done Nov 2, 2019
Finalize polyfill automation moved this from To do to Done Nov 2, 2019
ryzokuken added a commit that referenced this issue Nov 2, 2019
@ryzokuken
[api] rename inZone to inTimeZone
a26e451 
Rename `Absolute.prototype.inZone` and `DateTime.prototype.inZone` to
`Absolute.prototype.inTimeZone` and `DateTime.prototype.inTimeZone`
respectively, since Temporal.TimeZone now exists.

Fixes: #235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ryzokuken ryzokuken

Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
Finalize polyfill
  
Done
Finalize spec text
  
Done
Milestone
November Meeting
Development

Successfully merging a pull request may close this issue.

[api] rename inZone to inTimeZone ryzokuken/proposal-temporal
4 participants
@littledan @pipobscure @maggiepint @ryzokuken