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.

Sign up for GitHub

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

Editorial: Refactor CalendarFields/PrepareTemporalFields to ensure correct use #2823

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Editorial: Refactor CalendarFields/PrepareTemporalFields to ensure correct use #2823

merged 2 commits into from
Apr 26, 2024

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Apr 18, 2024

Closes: #2630

ptomato added 2 commits April 17, 2024 17:58
@ptomato
Editorial: Refactor simple case of CalendarFields + PrepareTemporalFi…
fafccf6 
…elds

This adds a new operation PrepareCalendarFields which consists of calling
the `fields` method of a calendar with a list of calendar property names,
appending a list of non-calendar property names to the result, followed by
calling PrepareTemporalFields to read all of the given properties from the
given object.

See: #2630
@ptomato
Editorial: Refactor full case of CalendarFields + PrepareTemporalFields
1267566 
This removes the remaining uses of CalendarFields by refactoring them into
PrepareCalendarFieldsAndFieldNames, which does the same thing as
PrepareCalendarFields but also returns the field names obtained from the
calendar, as well as the object returned from PrepareTemporalFields.

After this commit, it is no longer possible to call CalendarFields without
validating that there are no duplicates in the array returned from the
calendar's fields() method.

Closes: #2630
@ptomato ptomato mentioned this pull request Apr 18, 2024
Merged
@Ms2ger Ms2ger merged commit d967a01 into main Apr 26, 2024
5 checks passed
@Ms2ger Ms2ger deleted the 2630-refactor-calendarfields branch April 26, 2024 07:43
ptomato added a commit that referenced this pull request May 2, 2024
@ptomato
Editorial: Fix omitted parameter in PrepareCalendarFieldsAndFieldNames
798712a 
Thanks to Anba for spotting this. Overlooked in #2823. No need to change
reference code or tests; the reference code already has this.

Closes: #2831
Ms2ger pushed a commit that referenced this pull request May 3, 2024
@ptomato @Ms2ger
Editorial: Fix omitted parameter in PrepareCalendarFieldsAndFieldNames
9460dd8 
Thanks to Anba for spotting this. Overlooked in #2823. No need to change
reference code or tests; the reference code already has this.

Closes: #2831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jesus-Rojas Jesus-Rojas Jesus-Rojas approved these changes

@Ms2ger Ms2ger Ms2ger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reorganize PrepareTemporalFields and CalendarFields abstract operations
3 participants
@ptomato @Ms2ger @Jesus-Rojas