Skip to content

src: add fcntl.h include to node.cc#12540

Closed
bzoz wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:bartek-add-fcntl
Closed

src: add fcntl.h include to node.cc#12540
bzoz wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:bartek-add-fcntl

Conversation

@bzoz
Copy link
Copy Markdown
Contributor

@bzoz bzoz commented Apr 20, 2017

PR #11863 added _O_RDWR to node.cc which is defined in fcntl.h. This adds this include directly to node.cc to prevent build fails like in #12498

/cc @MylesBorins

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src
@bzoz
src: add fcntl.h include to node.cc
8c34036 
nodejs#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.
@bzoz bzoz added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. lts-watch-v4.x process Issues and PRs related to the process subsystem. labels Apr 20, 2017
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 20, 2017
@bzoz bzoz mentioned this pull request Apr 20, 2017
Merged
@gibfahn
Copy link
Copy Markdown
Member

gibfahn commented Apr 21, 2017

cc/ @nodejs/platform-windows
@MylesBorins
Copy link
Copy Markdown
Contributor

MylesBorins commented Apr 24, 2017

ci: https://ci.nodejs.org/job/node-test-pull-request/7625/
TimothyGu
TimothyGu reviewed Apr 24, 2017
View reviewed changes
Comment thread src/node.cc
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <fcntl.h>
Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Apr 24, 2017

Choose a reason for hiding this comment

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

Can you please maintain the alphabetical order? A comment detailing what macro is used like above for PATH_MAX would be helpful too.
@refack refack mentioned this pull request Apr 24, 2017
Closed
3 tasks
MylesBorins
MylesBorins approved these changes Apr 24, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@MylesBorins MylesBorins left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
@MylesBorins
Copy link
Copy Markdown
Contributor

MylesBorins commented Apr 24, 2017

Landed in 43c1b62
I took the liberty to alphabetize and include a comment as per @TimothyGu's request
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
43c1b62 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@eugeneo
Copy link
Copy Markdown
Contributor

eugeneo commented Apr 24, 2017

Commit 43c1b62 breaks cpp lint - there's an unwanted dot
@eugeneo
Copy link
Copy Markdown
Contributor

eugeneo commented Apr 24, 2017

9b4ae9f has the dot as well :)
@MylesBorins
Copy link
Copy Markdown
Contributor

MylesBorins commented Apr 24, 2017

@eugeneo BAHHHH #12626
@jasnell
Copy link
Copy Markdown
Member

jasnell commented Apr 24, 2017

I totally missed the dot also lol ... linting didn't complain about that one.
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
58230b4 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
fdb8975 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@refack refack mentioned this pull request Apr 24, 2017
Closed
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
c667e6e 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
117195a 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 24, 2017
@bzoz @MylesBorins
src: add fcntl.h include to node.cc
b5b78b1 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@refack refack mentioned this pull request Apr 25, 2017
Closed
evanlucas pushed a commit that referenced this pull request Apr 25, 2017
@bzoz @evanlucas
src: add fcntl.h include to node.cc
c02e12b 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@evanlucas evanlucas mentioned this pull request May 1, 2017
Merged
evanlucas pushed a commit that referenced this pull request May 1, 2017
@bzoz @evanlucas
src: add fcntl.h include to node.cc
06db34b 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 2, 2017
Merged
evanlucas pushed a commit that referenced this pull request May 2, 2017
@bzoz @evanlucas
src: add fcntl.h include to node.cc
520f876 
#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: #12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@bzoz @andrew749
src: add fcntl.h include to node.cc
d06b69b 
nodejs/node#11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

PR-URL: nodejs/node#12540
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. process Issues and PRs related to the process subsystem.