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

net,dns:move hasObserver out of perf function by theanarkh · Pull Request #43217 · nodejs/node · GitHub

/ node Public

net,dns:move hasObserver out of perf function - #43217

Merged
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
theanarkh:move_hasObserver_out_of_perf_function
Jun 9, 2022
ConversationCommits (1)ChecksFiles changed
Merged

net,dns:move hasObserver out of perf function#43217
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
theanarkh:move_hasObserver_out_of_perf_function

Conversation

theanarkh commented May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer.

See #43002

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Affected subsystem: net,dns

nodejs-github-bot commented May 27, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

nodejs-github-bot added dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels May 27, 2022
theanarkh force-pushed the move_hasObserver_out_of_perf_function branch from 689ad95 to 7cb6990 Compare May 27, 2022 01:40

theanarkh commented May 28, 2022

Copy link
Copy Markdown
Contributor Author

@mcollina Hello, can you help review this PR? Thanks !

mcollina approved these changes May 28, 2022
View reviewed changes

mcollina 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

lgtm

theanarkh force-pushed the move_hasObserver_out_of_perf_function branch 2 times, most recently from 896bf69 to 18b93f8 Compare May 31, 2022 17:59
github-actions Bot mentioned this pull request Jun 1, 2022
Open
17 tasks
net,dns: move hasObserver out of perf function
42c6e46 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer
theanarkh force-pushed the move_hasObserver_out_of_perf_function branch from 18b93f8 to 42c6e46 Compare June 1, 2022 04:39

theanarkh commented Jun 1, 2022

Copy link
Copy Markdown
Contributor Author

@mcollina @JungMinu Hi, can you please help trigger CI ? thanks !

mcollina approved these changes Jun 1, 2022
View reviewed changes

mcollina 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

lgtm

theanarkh commented Jun 1, 2022

Copy link
Copy Markdown
Contributor Author

lgtm

Thanks. can you please help trigger CI ?

This comment was marked as outdated.

Sign in to view
github-actions Bot mentioned this pull request Jun 2, 2022
Open
19 tasks

theanarkh commented Jun 2, 2022

Copy link
Copy Markdown
Contributor Author

@mcollina @JungMinu Hi, can you please help trigger CI again ? thanks !

This comment was marked as outdated.

Sign in to view

ShogunPanda commented Jun 2, 2022

Copy link
Copy Markdown
Contributor

@theanarkh I took care of it. Enjoy :)

theanarkh commented Jun 2, 2022

Copy link
Copy Markdown
Contributor Author

@theanarkh I took care of it. Enjoy :)

Thank you very much 😊.

github-actions Bot mentioned this pull request Jun 3, 2022
Open
19 tasks
RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 3, 2022

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
github-actions Bot mentioned this pull request Jun 4, 2022
Open
21 tasks

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

aduh95 commented Jun 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

test-net-connect-reset-until-connected is consistently failing on SmartOS CI (timeout).

theanarkh commented Jun 8, 2022

Copy link
Copy Markdown
Contributor Author

@RaisinTen @aduh95 @ShogunPanda @mcollina @JungMinu Hi, can you please help merge this PR ? Thanks !

RaisinTen commented Jun 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

test-net-connect-reset-until-connected is consistently failing on SmartOS CI (timeout).

That one seems to be passing in the latest CI run - https://ci.nodejs.org/job/node-test-pull-request/44373/, landing now.

RaisinTen added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2022
nodejs-github-bot merged commit 85f8821 into nodejs:master Jun 9, 2022

nodejs-github-bot commented Jun 9, 2022

Copy link
Copy Markdown
Collaborator

Landed in 85f8821

danielleadams pushed a commit that referenced this pull request Jun 11, 2022
net,dns: move hasObserver out of perf function
5661a4d 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: #43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams mentioned this pull request Jun 11, 2022
Merged
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
net,dns: move hasObserver out of perf function
9803b82 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: #43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
net,dns: move hasObserver out of perf function
64935a4 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: #43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
net,dns: move hasObserver out of perf function
d766cfa 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: #43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
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
net,dns: move hasObserver out of perf function
b0cbc1b 
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: nodejs/node#43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
JungMinu JungMinu approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL