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.

Sign up for GitHub

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

Jump to bottom

src: bench-statSync.js change fstat to fstatSync #36209

Closed
wants to merge 1 commit into from
Closed

src: bench-statSync.js change fstat to fstatSync #36209

wants to merge 1 commit into from

Conversation

ntedgi
Copy link
Contributor

@ntedgi ntedgi commented Nov 21, 2020 •
edited by aduh95

Changed fstat to fstatSync at bench-statSync.js as mentioned in the issue #36199

Fixes: #36199

  • make -j4 test (UNIX)
  • tests and/or benchmarks are included

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. fs Issues and PRs related to the fs subsystem / file system. labels Nov 21, 2020
@ntedgi ntedgi changed the title src change fstat to fstatSync src: bench-statSync.js src: bench-statSync.js change fstat to fstatSync Nov 21, 2020
@aduh95 aduh95 requested a review from mscdex November 21, 2020 12:55
@gireeshpunathil
Copy link
Member

looks like same changes proposed by two folks: #36206 and this one. @Narasimha1997 and @ntedgi - would you discuss among yourself please? those who wants to pick up other work items, please review https://github.com/nodejs/node/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22

@ntedgi
Copy link
Contributor Author

ntedgi commented Nov 23, 2020

@gireeshpunathil @Narasimha1997 open is PR first so you can close mine
maybe next time
thanks for the review @aduh95 and @gireeshpunathil

@gireeshpunathil
Copy link
Member

@ntedgi - thanks!

Can I propose you to have a look at https://nodejs.org/dist/latest-v15.x/docs/api/buffer.html#buffer_static_method_buffer_isbuffer_obj and see if you can add an example code (that is missing there) , following how it is done in the neighboring APIs? the doc for the APIs are in /doc/api/buffer.md . (We want to make sure your intent of contributing to Node.js is achieved!)

@gireeshpunathil
Copy link
Member

closing in favor of #36206

@ntedgi ntedgi mentioned this pull request Nov 23, 2020
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@aduh95 aduh95 aduh95 approved these changes

@kurdenes kurdenes kurdenes approved these changes

@mscdex mscdex Awaiting requested review from mscdex

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

benchmark: dead code
5 participants
@ntedgi @gireeshpunathil @aduh95 @kurdenes @nodejs-github-bot