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

events: convert errorMonitor to normal property by Flarna · Pull Request #31848 · nodejs/node · GitHub

/ node Public

events: convert errorMonitor to normal property - #31848

Closed
Flarna wants to merge 1 commit into
nodejs:masterfrom
dynatrace-oss-contrib:events-error-monitor
Closed

events: convert errorMonitor to normal property#31848
Flarna wants to merge 1 commit into
nodejs:masterfrom
dynatrace-oss-contrib:events-error-monitor

Conversation

Flarna commented Feb 18, 2020
edited
Loading

Copy link
Copy Markdown
Member

Convert property errorMonitor to a normal property as non-writable caused unwanted side effects.

Refs: #30932 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

nodejs-github-bot added the events Issues and PRs related to the events subsystem / EventEmitter. label Feb 18, 2020
Comment thread lib/events.js Outdated
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: nodejs#30932 (comment)
Flarna force-pushed the events-error-monitor branch from d1b498c to e00a3fb Compare February 18, 2020 15:33
Flarna changed the title events: convert errorMonitor to getter events: convert errorMonitor to normal property Feb 21, 2020

Flarna commented Feb 25, 2020

Copy link
Copy Markdown
Member Author

Friendly ping, would be nice if someone could move this forward.

mmarchini added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 26, 2020

Copy link
Copy Markdown
Collaborator

Flarna commented Feb 28, 2020

Copy link
Copy Markdown
Member Author

Is there anything I can do for CI? I haven't found a way yet to retrigger it...

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Flarna commented Mar 2, 2020

Copy link
Copy Markdown
Member Author

Is there something I can do for CI? Does it help if I rebase?

Copy link
Copy Markdown
Contributor

I'll land this, the only failure is

08:29:22 not ok 650 known_issues/test-vm-timeout-escape-queuemicrotask
08:29:22   ---
08:29:22   duration_ms: 1.242
08:29:22   severity: fail
08:29:22   stack: |-
08:29:22   ...

which is a known flaky (#32048) and won't be fixed (#31980).

mmarchini pushed a commit that referenced this pull request Mar 2, 2020
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: #30932 (comment)

PR-URL: #31848
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Copy link
Copy Markdown
Contributor

Landed in ed8007a

mmarchini closed this Mar 2, 2020
Flarna deleted the events-error-monitor branch March 2, 2020 22:13
MylesBorins pushed a commit that referenced this pull request Mar 4, 2020
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: #30932 (comment)

PR-URL: #31848
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins mentioned this pull request Mar 4, 2020

targos commented Apr 20, 2020

Copy link
Copy Markdown
Member

Depends on #30932 to land on v12.x

ljharb commented Apr 20, 2020

Copy link
Copy Markdown
Member

@targos to clarify; if #30932 doesn't land on 12.x, this doesn't need to either - but if it does, this must :-)

targos commented Apr 20, 2020

Copy link
Copy Markdown
Member

Yes. Sorry if I'm not giving enough details in these messages, but I have about 300 semver-patch commits to review to prepare the next v12.x release.

Flarna commented Apr 20, 2020

Copy link
Copy Markdown
Member Author

There is a backport PR which combines both: #32004
As #30932 is semver-minor the backport PR doesn't fit into a patch release.

targos added backport-open-v12.x and removed backport-blocked-v12.x author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Apr 20, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: nodejs#30932 (comment)

PR-URL: nodejs#31848
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Apr 28, 2020
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: #30932 (comment)

PR-URL: #31848
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@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

events Issues and PRs related to the events subsystem / EventEmitter.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants


Back | FazBrowse Home | New Git URL