Skip to content

Extend usage of ES6 arrow functions and default params. NFC #15758

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
Merged
merged 1 commit into from
Jan 8, 2022
Merged

Extend usage of ES6 arrow functions and default params. NFC #15758

merged 1 commit into from
Jan 8, 2022

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 13, 2021 •
edited
Loading

See #11984

curiousdannii
curiousdannii reviewed Dec 13, 2021
View reviewed changes
juj
juj reviewed Dec 13, 2021
View reviewed changes
juj
juj reviewed Dec 13, 2021
View reviewed changes
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 13, 2021

Thanks @juj, I think are you correct in both cases.. sorry I didn't mean for this PR to be ready for review yet, its part of a PR chain that I hope to land in small pieces.

@sbc100 sbc100 changed the base branch from main to use_es6 December 13, 2021 20:09
@sbc100 sbc100 mentioned this pull request Dec 13, 2021
Closed
@sbc100 sbc100 mentioned this pull request Dec 13, 2021
Merged
@sbc100 sbc100 force-pushed the use_es6 branch 2 times, most recently from 1062e99 to a791242 Compare December 15, 2021 03:35
@sbc100 sbc100 changed the title [WIP] Second round of ES6 usage in core libraries Second round of ES6 usage in core libraries Dec 15, 2021
curiousdannii
curiousdannii reviewed Dec 19, 2021
View reviewed changes
@sbc100 sbc100 force-pushed the use_es6 branch 2 times, most recently from 47681b0 to c1f5e24 Compare December 20, 2021 21:15
Base automatically changed from use_es6 to main December 21, 2021 18:51
@sbc100 sbc100 force-pushed the use_es6_part2 branch 3 times, most recently from 2feb412 to 0087aff Compare December 29, 2021 11:52
@sbc100 sbc100 changed the title Second round of ES6 usage in core libraries Extend usage of ES6 arrow functions and default params. NFC Dec 29, 2021
juj
juj requested changes Jan 4, 2022
View reviewed changes
Copy link
Collaborator

@juj juj left a comment

Choose a reason for hiding this comment

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

lgtm after the few this vs that fixes.

@sbc100 sbc100 force-pushed the use_es6_part2 branch 3 times, most recently from cfa466c to 65d87e5 Compare January 8, 2022 14:02
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 8, 2022

Updated the PR to include code size wins.

@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 8, 2022

Could you unlock/lgtm?

@sbc100 sbc100 enabled auto-merge (squash) January 8, 2022 14:57
@sbc100 sbc100 disabled auto-merge January 8, 2022 18:32
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 8, 2022

Windows failure looks like an unrelated/intermittent failure of the chocolaty packaging system to install dependencies.

@sbc100 sbc100 merged commit c0f1058 into main Jan 8, 2022
@sbc100 sbc100 deleted the use_es6_part2 branch January 8, 2022 18:33
sbc100 added a commit that referenced this pull request Jan 18, 2022
@sbc100
library_async.js: Use arrow functions where possible. NFC
956c10f 
Followup to #15758
@sbc100 sbc100 mentioned this pull request Jan 18, 2022
Merged
sbc100 added a commit that referenced this pull request Jan 18, 2022
@sbc100
library_async.js: Use arrow functions where possible. NFC (#16051)
a02b17c 
Followup to #15758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juj juj juj approved these changes

+1 more reviewer

@curiousdannii curiousdannii curiousdannii left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sbc100 @curiousdannii @juj