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

events: improve Event compatibility #43461

Merged
merged 1 commit into from
Jul 8, 2022
Merged

events: improve Event compatibility #43461

merged 1 commit into from
Jul 8, 2022

Conversation

daeyeon
Copy link
Member

@daeyeon daeyeon commented Jun 17, 2022

This fixes the Event constructor to improve Event Web API compatibility.

The test added was written by referring to wpt@dom/events/Event-constructors.any.js

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 17, 2022
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 17, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44658/

lpinca
lpinca approved these changes Jun 17, 2022
View reviewed changes
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

Nit: can you please rename test-event-whatwg-constructors.js to test-whatwg-event-constructors.js? For consistency with the existing test-whatwg-url-constructor.js.

@github-actions github-actions bot mentioned this pull request Jun 18, 2022
Open
34 tasks
@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

It looks better for consistency. Applied the suggestion. Thanks!

Actually, it was named by following test-eventtarget-whatwg-xxx.jss. 😂 Maybe worth renaming such files to avoid confusion.

@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from 49682ce to a4b26cb Compare June 18, 2022 01:34
@daeyeon
events: improve Event compatibility
2d372b3 
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from a4b26cb to b9a0f37 Compare June 18, 2022 01:51
@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

I squashed the fixup commits and added a commit to rename the files mentioned above.

@lpinca PTAL, and, if it's reasonable, could you remove the commit-queue-squash tag?

@lpinca
Copy link
Member

lpinca commented Jun 18, 2022

@daeyeon can you please move events: rename test-eventtarget-whatwg-xxx.js to a separate PR?

@daeyeon daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from b9a0f37 to 2d372b3 Compare June 18, 2022 05:54
@daeyeon daeyeon mentioned this pull request Jun 18, 2022
Merged
@daeyeon
Copy link
Member Author

daeyeon commented Jun 18, 2022

@lpinca Thanks for the review, updated!

aduh95 pushed a commit that referenced this pull request Jun 18, 2022
@daeyeon
test: rename test-eventtarget-whatwg-*.js
f209aee 
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This was referenced Jun 19, 2022
Open
Open
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44996/

@github-actions github-actions bot mentioned this pull request Jul 4, 2022
Open
23 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45130/

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2022
@nodejs-github-bot nodejs-github-bot merged commit 3f7f62f into nodejs:main Jul 8, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 3f7f62f

targos pushed a commit that referenced this pull request Jul 12, 2022
@daeyeon @targos
test: rename test-eventtarget-whatwg-*.js
3b0703f 
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
@daeyeon @targos
events: improve Event compatibility
061ed0e 
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Jul 12, 2022
Merged
@daeyeon daeyeon deleted the main.web.event-220617.Fri.3c24 branch July 12, 2022 14:05
targos pushed a commit that referenced this pull request Jul 18, 2022
@daeyeon @targos
test: rename test-eventtarget-whatwg-*.js
ea3d07b 
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
@daeyeon @targos
test: rename test-eventtarget-whatwg-*.js
4ff1c32 
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43467
Refs: #43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
@daeyeon @targos
events: improve Event compatibility
0167988 
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@daeyeon @guangwong
test: rename test-eventtarget-whatwg-*.js
d42a190 
This renames some test filenames to align with the existing
`parallel/test-whatwg-*.js` for consistency.

According to the comment in each file, they are seemingly manually
ported from the `wpt@dom/events` tests.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43467
Refs: nodejs/node#43461
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@daeyeon @guangwong
events: improve Event compatibility
324b833 
This fixes `Event` constructor to improve `Event Web API`
compatibility.

The test added was written by referring to
`wpt@dom/events/Event-constructors.any.js`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mscdex mscdex mscdex left review comments

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@aduh95 aduh95 aduh95 approved these changes

@LiviaMedeiros LiviaMedeiros LiviaMedeiros approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@daeyeon @nodejs-github-bot @lpinca @mscdex @jasnell @aduh95 @LiviaMedeiros