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

Don't call GetMethod on calendar String values #2547

Closed
anba opened this issue Apr 12, 2023 · 1 comment
Closed

Don't call GetMethod on calendar String values #2547

anba opened this issue Apr 12, 2023 · 1 comment
Assignees

Comments

@anba
Copy link
Contributor

anba commented Apr 12, 2023

GetMethod shouldn't be called on calendar String values in the following operations:

  • UnbalanceDurationRelative
  • BalanceDurationRelative
  • AddDuration
  • RoundDuration
@justingrant
Copy link
Collaborator

Meeting 2023-04-13: This was a bug in the spec text of #2482 that wasn't caught before that PR was merged a few days ago, so we'll treat it as if it were a review comment on that PR and fix it now.

@ptomato ptomato self-assigned this Apr 14, 2023
ptomato added a commit that referenced this issue Apr 14, 2023
When GetMethod is used to lookup the dateAdd or dateUntil method of a
calendar, we need to account for the case where the calendar is a builtin
calendar represented by a string. This was an oversight in #2482.

In hindsight, the lookups of dateAdd and dateUntil in
BalanceDurationRelative should have been grouped together as they were in
UnbalanceDurationRelative. But since that will be addressed anyway by the
Calendar Method Records in the yet-to-be-merged PR #2519, I'm not planning
to change that here.

Closes: #2547
ptomato added a commit that referenced this issue Apr 18, 2023
When GetMethod is used to lookup the dateAdd or dateUntil method of a
calendar, we need to account for the case where the calendar is a builtin
calendar represented by a string. This was an oversight in #2482.

In hindsight, the lookups of dateAdd and dateUntil in
BalanceDurationRelative should have been grouped together as they were in
UnbalanceDurationRelative. But since that will be addressed anyway by the
Calendar Method Records in the yet-to-be-merged PR #2519, I'm not planning
to change that here.

Closes: #2547
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

No branches or pull requests

3 participants