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

fs: move definition from header file #36256

Merged
merged 1 commit into from
Nov 29, 2020
Merged

fs: move definition from header file #36256

merged 1 commit into from
Nov 29, 2020

Conversation

yashLadha
Copy link
Contributor

@yashLadha yashLadha commented Nov 25, 2020 •
edited
Loading

Move definition of method from header file.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-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. labels Nov 25, 2020
@yashLadha yashLadha changed the title fs: remove duplicate method for binding_data fs: move definition from header file Nov 25, 2020
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 25, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 25, 2020
@nodejs-github-bot

This comment has been minimized.

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

nodejs-github-bot commented Nov 25, 2020 •
edited by aduh95
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/34557/ ✅

@yashLadha @Trott
fs: move method definition from header
9bf12df 
We are defining the definition of function in the header file itself. It
should be defined in the source file instead of the header file.

PR-URL: nodejs#36256
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott Trott merged commit 9bf12df into nodejs:master Nov 29, 2020
@Trott
Copy link
Member

Trott commented Nov 29, 2020

Landed in 9bf12df

danielleadams pushed a commit that referenced this pull request Dec 7, 2020
@yashLadha @danielleadams
fs: move method definition from header
537e5cb 
We are defining the definition of function in the header file itself. It
should be defined in the source file instead of the header file.

PR-URL: #36256
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Dec 7, 2020
Merged
cjihrig pushed a commit to cjihrig/node that referenced this pull request Dec 8, 2020
@yashLadha @cjihrig
fs: move method definition from header
b0157f4 
We are defining the definition of function in the header file itself. It
should be defined in the source file instead of the header file.

PR-URL: nodejs#36256
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
@yashLadha @targos
fs: move method definition from header
4552436 
We are defining the definition of function in the header file itself. It
should be defined in the source file instead of the header file.

PR-URL: #36256
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@lpinca lpinca lpinca 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yashLadha @nodejs-github-bot @Trott @jasnell @lpinca @aduh95