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.

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

test: refactor remove repeated execution index.js #839

Conversation

RaisinTen
Copy link
Contributor

@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
Contributor Author

Can someone please review this?

@mhdawson
Copy link
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
Contributor Author

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

@mhdawson
Copy link
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
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
Contributor Author

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

@RaisinTen
Copy link
Contributor Author

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

@mhdawson
Copy link
Member

mhdawson commented Jan 7, 2021

The calendar time should be correct. 11:00 ET

@RaisinTen
Copy link
Contributor 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
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
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: NickNaso <[email protected]>
@mhdawson
Copy link
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: NickNaso <[email protected]>
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: NickNaso <[email protected]>
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: NickNaso <[email protected]>
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: NickNaso <[email protected]>
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