FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

events: improve `Event` compatibility by daeyeon · Pull Request #43461 · nodejs/node · GitHub

/ node Public

events: improve Event compatibility - #43461

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
daeyeon:main.web.event-220617.Fri.3c24
Jul 8, 2022
Merged

events: improve Event compatibility#43461
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
daeyeon:main.web.event-220617.Fri.3c24

Conversation

daeyeon commented Jun 17, 2022

Copy link
Copy Markdown
Member

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 added the needs-ci PRs that need a full CI run. label Jun 17, 2022
Comment thread lib/internal/event_target.js Outdated
aduh95 added author ready PRs that have at least one approval, no outstanding review comments, 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 Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022

Copy link
Copy Markdown
Collaborator

lpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

daeyeon commented Jun 18, 2022

Copy link
Copy Markdown
Member Author

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 force-pushed the main.web.event-220617.Fri.3c24 branch from 49682ce to a4b26cb Compare June 18, 2022 01:34
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 force-pushed the main.web.event-220617.Fri.3c24 branch from a4b26cb to b9a0f37 Compare June 18, 2022 01:51

daeyeon commented Jun 18, 2022

Copy link
Copy Markdown
Member Author

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 commented Jun 18, 2022

Copy link
Copy Markdown
Member

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

daeyeon force-pushed the main.web.event-220617.Fri.3c24 branch from b9a0f37 to 2d372b3 Compare June 18, 2022 05:54

daeyeon commented Jun 18, 2022

Copy link
Copy Markdown
Member Author

@lpinca Thanks for the review, updated!

aduh95 pushed a commit that referenced this pull request Jun 18, 2022
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>
aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2022
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 merged commit 3f7f62f into nodejs:main Jul 8, 2022

Copy link
Copy Markdown
Collaborator

Landed in 3f7f62f

targos pushed a commit that referenced this pull request Jul 12, 2022
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
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>
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
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
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
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>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, 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

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL