Skip to content

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

Closed
ntedgi wants to merge 1 commit intonodejs:masterfrom
ntedgi:benchmark-dead-code-change-fstat-to-fstatsync
Closed

src: bench-statSync.js change fstat to fstatSync#36209
ntedgi wants to merge 1 commit intonodejs:masterfrom
ntedgi:benchmark-dead-code-change-fstat-to-fstatsync

Conversation

@ntedgi
Copy link
Contributor

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

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

gireeshpunathil commented Nov 23, 2020

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

gireeshpunathil commented Nov 23, 2020

@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

gireeshpunathil commented Nov 23, 2020

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

@mscdex mscdex Awaiting requested review from mscdex

+1 more reviewer

@kurdenes kurdenes kurdenes approved these changes

Reviewers whose approvals may not affect merge requirements

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