Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Comments

docs: Updated node-process-nexttick.en.md #3266

Merged
aymen94 merged 2 commits intonodejs:mainfrom
abhishekguha95:patch-1
Mar 13, 2023
Merged

docs: Updated node-process-nexttick.en.md #3266
aymen94 merged 2 commits intonodejs:mainfrom
abhishekguha95:patch-1

Conversation

@abhishekguha95
Copy link
Contributor

Description

Changed the invocation sequence of setTimeout , setImmediate and output logs to maintain output numbering order. ie. 1 -> 2 -> 3 -> 4.

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run lint:js -- --fix and/or npm run lint:md -- --fix for my JavaScript and/or Markdown changes.
    • This is important as most of the cases your code changes might not be correctly linted
  • I have run npm run test to check if all tests are passing, and/or npm run test -- -u to update snapshots if I created and/or updated React Components.
  • I have checked that the build works locally and that npm run build work fine.
  • I've covered new added functionality with unit tests if necessary.

abhishekguha95 and others added 2 commits March 13, 2023 17:05
setImmediate invocation should be before the setTimeout invocation to maintain the order of the logged numbers.

Signed-off-by: Abhishek Guha <66299749+abhishekguha95@users.noreply.github.com>
@abhishekguha95 abhishekguha95 changed the title Updated node-process-nexttick.en.md docs: Updated node-process-nexttick.en.md Mar 13, 2023
Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your first contribution!

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

The change is code is good. But output isn't good.
So I'm not in favor of this change.

@ovflowd
Copy link
Member

ovflowd commented Mar 13, 2023

The change is code is good. But output isn't good.
So I'm not in favor of this change.

Sorry, what is wrong with the output?

@AugustinMauroy
Copy link
Member

Sorry, what is wrong with the output?

Oups nothing error with NVM. So I take to old version.

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

LGTM 👍! Thanks for your first contribution and welcome.

@aymen94
Copy link
Member

aymen94 commented Mar 13, 2023

LGTM 👌, Thanks for your first contribution!^_^

@aymen94 aymen94 merged commit dba1d76 into nodejs:main Mar 13, 2023
@abhishekguha95
Copy link
Contributor Author

Thank you for the kind words! This was my 1st open-source contribution. I'm grateful for the opportunity and look forward to continuing to do so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants