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: Change ValidateTypedArray to not return the buffer #2376

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Apr 8, 2021

See #2355 (comment)

Downstream: No uses of ValidateTypedArray in WebIDL, HTML, Intl.

Note that ValidateIntegerTypedArray also returns the buffer, so you might want that handled similarly.

Also, ValidateAtomicAccess returns a value, but changing it to not return that value would be more disruptive. Perhaps it should change its name to something that emphasizes the value it computes, rather than any validation it does.

@ljharb ljharb requested review from syg, michaelficarra, bakkot and a team April 8, 2021 13:44
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

lgtm.

+1 on ValidateIntegerTypedArray and ValidateAtomicAccess should change similarly or rename, as you suggested.

@syg syg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jun 2, 2021
@ljharb ljharb force-pushed the ValidateTypedArray branch from b083b70 to f7264bb Compare June 2, 2021 21:36
@ljharb ljharb merged commit f7264bb into tc39:master Jun 2, 2021
@jmdyck jmdyck deleted the ValidateTypedArray branch June 3, 2021 00:06
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change 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.

4 participants