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: remove deadcode in node_file.cc #38693

Closed
wants to merge 1 commit into from
Closed

src: remove deadcode in node_file.cc #38693

wants to merge 1 commit into from

Conversation

juanarbol
Copy link
Member

The implementation of the fs.lchmod method has been moved to JS-land

@github-actions github-actions bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 15, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 15, 2021
cjihrig
cjihrig approved these changes May 15, 2021
View reviewed changes
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd refer to it as commented out code in the commit message.

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@juanarbol
src: remove commented code in node_file.cc
b0722e7 
The implementation of the `fs.lchmod` method has been moved to JS-land
@juanarbol juanarbol force-pushed the juan/remove-cc-deadcode branch from 7d814c9 to b0722e7 Compare May 16, 2021 16:13
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 21, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/38246/ 💚

@jasnell
Copy link
Member

jasnell commented May 21, 2021

Landed in c9269a4

@jasnell jasnell closed this May 21, 2021
jasnell pushed a commit that referenced this pull request May 21, 2021
@juanarbol @jasnell
src: remove commented code in node_file.cc
c9269a4 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: #38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
danielleadams pushed a commit that referenced this pull request May 31, 2021
@juanarbol @danielleadams
src: remove commented code in node_file.cc
c576311 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: #38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
@danielleadams danielleadams mentioned this pull request May 31, 2021
Merged
richardlau pushed a commit that referenced this pull request Jul 16, 2021
@juanarbol @richardlau
src: remove commented code in node_file.cc
6e51f30 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: #38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
richardlau pushed a commit that referenced this pull request Jul 19, 2021
@juanarbol @richardlau
src: remove commented code in node_file.cc
f50d459 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: #38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
@juanarbol @richardlau
src: remove commented code in node_file.cc
2c11d3e 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: #38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
@richardlau richardlau mentioned this pull request Jul 20, 2021
Merged
@juanarbol juanarbol deleted the juan/remove-cc-deadcode branch August 26, 2021 17:54
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@juanarbol @foxxyz
src: remove commented code in node_file.cc
76d1eb0 
The implementation of the `fs.lchmod` method has been moved to JS-land

PR-URL: nodejs#38693
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaisinTen RaisinTen RaisinTen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@antsmartian antsmartian antsmartian approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@juanarbol @nodejs-github-bot @jasnell @Trott @addaleax @antsmartian @lpinca @cjihrig @RaisinTen