Skip to content

src: tidy up js_native_api.* headers #26884

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 5, 2019
Merged

src: tidy up js_native_api.* headers #26884

merged 1 commit into from
Apr 5, 2019

Conversation

gengjiawen
Copy link
Member

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

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 24, 2019
addaleax
addaleax reviewed Mar 24, 2019
View reviewed changes
@gengjiawen
Copy link
Member Author

gengjiawen commented Mar 24, 2019 •
edited
Loading

Any idea why can't find cstdint in test ? https://travis-ci.org/gengjiawen/node/jobs/510606413.

/home/travis/build/gengjiawen/node/src/js_native_api_types.h:4:19: fatal error: cstdint: No such file or directory
 #include <cstdint>
                   ^
compilation terminated.
make[2]: *** [Release/obj.target/binding/../entry_point.o] Error 1
    at ChildProcess.exithandler (child_process.js:298:12)
    at ChildProcess.emit (events.js:194:13)
    at maybeClose (internal/child_process.js:1000:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:265:5)
Makefile:410: recipe for target 'test/js-native-api/.buildstamp' failed
make[1]: *** [test/js-native-api/.buildstamp] Error 1
Makefile:291: recipe for target 'test' failed
make: *** [test] Error 2

addaleax
addaleax reviewed Mar 24, 2019
View reviewed changes
@gengjiawen gengjiawen force-pushed the tidyup_js_native_headers branch 2 times, most recently from d62e49e to d8829a6 Compare March 25, 2019 14:00
@BridgeAR BridgeAR requested review from addaleax and refack March 27, 2019 10:37
@gengjiawen gengjiawen force-pushed the tidyup_js_native_headers branch from d8829a6 to 42baf72 Compare March 28, 2019 14:21
@gengjiawen gengjiawen mentioned this pull request Mar 30, 2019
Closed
2 tasks
refack
refack reviewed Mar 30, 2019
View reviewed changes
@gengjiawen gengjiawen force-pushed the tidyup_js_native_headers branch from 42baf72 to 1f059e3 Compare March 31, 2019 06:58
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 1, 2019
@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung joyeecheung mentioned this pull request Apr 2, 2019
Closed
@gengjiawen gengjiawen force-pushed the tidyup_js_native_headers branch from 1f059e3 to 5073335 Compare April 2, 2019 12:32
BridgeAR
BridgeAR reviewed Apr 4, 2019
View reviewed changes
@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 4, 2019
@refack
Copy link
Contributor

refack commented Apr 5, 2019 •
edited
Loading

Current change is semver-neutral, hence it is safe to backport.

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3178/

@gengjiawen @refack
src: add NOLINT to js_native_.*
d0e2650 
* add filter to not lint NOLINT rules

PR-URL: nodejs#26884
Reviewed-By: Refael Ackermann <refack@gmail.com>
@refack refack force-pushed the tidyup_js_native_headers branch from 7bac9ba to d0e2650 Compare April 5, 2019 12:59
@refack refack merged commit d0e2650 into nodejs:master Apr 5, 2019
@beevelop beevelop mentioned this pull request Apr 23, 2019
Closed
This was referenced Apr 23, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@refack refack refack approved these changes

@BridgeAR BridgeAR BridgeAR left review comments

@addaleax addaleax Awaiting requested review from addaleax

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gengjiawen @nodejs-github-bot @refack @addaleax @BridgeAR