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: Expand the use of 'type' syntax #2691

Merged
merged 4 commits into from Mar 17, 2022

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Mar 15, 2022

Similar to PR #2602, which used 'type' syntax when declaring record fields, this PR uses it when declaring:

  • the require/optional properties of interfaces, and
  • the internal slots of objects.

Also, there's a bonus commit that fixes up the field types of a recently added record.

spec.html Outdated
@@ -42881,7 +42896,7 @@ <h1>The <i>IteratorResult</i> Interface</h1>
*"done"*
</td>
<td>
Either *true* or *false*.
either *true* or *false*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
either *true* or *false*
a Boolean

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me.

spec.html Outdated
@@ -44510,6 +44558,9 @@ <h1>Properties of Generator Instances</h1>
<td>
[[GeneratorBrand]]
</td>
<td>
a brand
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually restrict this field in any way? Is it more useful to say this than "anything"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Within the spec, it's a String or ~empty~, so I'd be inclined to put that in. (Or are we allowing for the possibility of spec-external uses that need a wider type?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a String or ~empty~ works for me.

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise.

@jmdyck jmdyck force-pushed the use_type_syntax_more branch 2 times, most recently from 4fc9df9 to aaae37c Compare March 17, 2022 19:47
@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Mar 17, 2022
In tables that 'declare' the required/optional properties of an interface,
reword the 'types' (in the 'Value' column)
to use the same (or similar) style as the parameters/return
of abstract operations.
In tables that 'declare' the internal slots of an object,
reword the 'types' (in the 'Type' column)
to use the same (or similar) style as the parameters/return
of abstract operations.
In tables that 'declare' the internal slots of an object,
where there are only 2 columns ("Internal Slot" and "Description"),
add a "Type" column.
... in the Iterator Record Fields table.
@ljharb ljharb merged commit 8d9522c into tc39:main Mar 17, 2022
@jmdyck jmdyck deleted the use_type_syntax_more branch March 21, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change establishes editorial conventions ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants