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

Editorial request add Calendar prefix to 12.1.24|25|26 *FromFields #1670

Closed
FrankYFTang opened this issue Jul 22, 2021 · 1 comment · Fixed by #2140
Closed

Editorial request add Calendar prefix to 12.1.24|25|26 *FromFields #1670

FrankYFTang opened this issue Jul 22, 2021 · 1 comment · Fixed by #2140
Assignees
Labels
editorial spec-text Specification text involved
Milestone

Comments

@FrankYFTang
Copy link
Contributor

This is just an Editorial request to make our spec easier to follow
Currently we have many AOs named CalendarXXXX which all have pattern like

1. Let fields be ? GetMethod(calendar, "methodnames").
...
a. Set fieldsArray to ? Call(fields, calendar, « fieldsArray »).

in 12.1.5 - 12.1.8
or pattern like

2. Return ? Invoke(calendar, "methodnames", « dateLike »).

in 12.1.9 - 12.1.20

But three of the following also use Invoke but without the Calendar prefix
https://tc39.es/proposal-temporal/#sec-temporal-datefromfields
12.1.24 DateFromFields ( calendar, fields, options )
12.1.25 YearMonthFromFields ( calendar, fields [ , options ] )
12.1.26 MonthDayFromFields ( calendar, fields [ , options ] )

Could we rename it to follow the same naming patter as
12.1.24 CalendarDateFromFields ( calendar, fields, options )
12.1.25 CalendarYearMonthFromFields ( calendar, fields [ , options ] )
12.1.26 CalendarMonthDayFromFields ( calendar, fields [ , options ] )

to make it easier to understand.

Not very important but I think it may make the spec easier to follow.

@ptomato ptomato added editorial spec-text Specification text involved labels Jul 26, 2021
@ptomato ptomato added this to the Next milestone Jul 26, 2021
@ptomato ptomato self-assigned this Apr 12, 2022
@ptomato
Copy link
Collaborator

ptomato commented Apr 12, 2022

I may as well get this out of the way before refactoring the Calendar spec in #1808...

ptomato added a commit that referenced this issue Apr 13, 2022
Naming these in line with all the other CalendarFoo operations that call
the 'foo' method of the given calendar, makes them easier to understand
when reading the spec text.

Closes: #1670
Ms2ger pushed a commit that referenced this issue Apr 13, 2022
Naming these in line with all the other CalendarFoo operations that call
the 'foo' method of the given calendar, makes them easier to understand
when reading the spec text.

Closes: #1670
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial spec-text Specification text involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants