Skip to content

test: refactor remove repeated execution index.js#839

Closed
RaisinTen wants to merge 1 commit intonodejs:masterfrom
RaisinTen:test/refactor-to-remove-repetition-index.js
Closed

test: refactor remove repeated execution index.js#839
RaisinTen wants to merge 1 commit intonodejs:masterfrom
RaisinTen:test/refactor-to-remove-repetition-index.js

Conversation

@RaisinTen
Copy link
Copy Markdown
Member

@RaisinTen RaisinTen commented Nov 20, 2020

Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the testModule is done by
the child thread.

@RaisinTen RaisinTen force-pushed the test/refactor-to-remove-repetition-index.js branch 3 times, most recently from f03f433 to c472036 Compare December 25, 2020 13:04
@RaisinTen
Copy link
Copy Markdown
Member Author

Can someone please review this?

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Jan 4, 2021

@RaisinTen maybe you can come to the N-API team meeting this Friday to walk us through it. It's a "nice to have" and obviously nobody has had time to get around to it so that might help move it forward.

@RaisinTen
Copy link
Copy Markdown
Member Author

@mhdawson that would be great! I plan to join the Hangouts link. Is that okay? :)

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Jan 5, 2021

@RaisinTen absolutely (although its zoom). Just in case you need it: https://zoom.us/j/363665824
and the meeting is Friday at 11 EST.

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Jan 5, 2021

@RaisinTen and to be 100% clear, the only option is zoom even though there are other links in the calendar entry, I've just updated it so that its clearer that they are generally not used.

@RaisinTen
Copy link
Copy Markdown
Member Author

@mhdawson thank you for updating it. Looking forward to the meet.

@RaisinTen
Copy link
Copy Markdown
Member Author

@mhdawson I was a little confused about the timing. Is the node calendar timing yet to be updated?

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Jan 7, 2021

The calendar time should be correct. 11:00 ET

@RaisinTen
Copy link
Copy Markdown
Member Author

@mhdawson hmm, when I saved the event to my calendar, it showed the correct time indeed. However, the time shown at nodejs.org/calendar is still different. What time standard does it follow?

Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.
@RaisinTen RaisinTen force-pushed the test/refactor-to-remove-repetition-index.js branch from c472036 to 451d418 Compare January 8, 2021 08:28
Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

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

LGTM

mhdawson pushed a commit that referenced this pull request Jan 8, 2021
Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.

PR-URL: #839
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Jan 8, 2021

Landed as 744705f

@mhdawson mhdawson closed this Jan 8, 2021
@RaisinTen RaisinTen deleted the test/refactor-to-remove-repetition-index.js branch January 9, 2021 08:04
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.

PR-URL: nodejs/node-addon-api#839
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.

PR-URL: nodejs/node-addon-api#839
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.

PR-URL: nodejs/node-addon-api#839
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
Since the only job of the main thread is to supply the correct
command-line args to the child thread, here is a refactor to do
just that. The generation and execution of the `testModule` is done by
the child thread.

PR-URL: nodejs/node-addon-api#839
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants