Skip to content

Conversation

@danbev
Copy link
Contributor

As indicated by the FIXME comment, this macro guard is no longer needed.

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

src

As indicated by the FIXME comment, this macro guard is no longer needed.
@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 25, 2017
@danbev
Copy link
ContributorAuthor

@addaleax
Copy link
Member

LGTM

Landed in 5bb06e8

addaleax pushed a commit that referenced this pull request Apr 27, 2017
As indicated by the FIXME comment, this macro guard is no longer needed. PR-URL: #12638 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas
Copy link
Contributor

This is landing on v7.x-staging, but causes a compilation error.

Undefined symbols for architecture x86_64: "node::Abort()", referenced from: node::inspector::InspectorSocketServer::HandshakeCallback(node::inspector::InspectorSocket*, node::inspector::inspector_handshake_event, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) in inspector_socket_server.o "node::Assert(char const* const (*) [4])", referenced from: UtilTest_Malloc_Test::TestBody() in util.o UtilTest_Calloc_Test::TestBody() in util.o UtilTest_MaybeStackBuffer_Test::TestBody() in util.o node::MaybeStackBuffer<unsigned char, 1024ul>::SetLength(unsigned long) in util.o node::MaybeStackBuffer<unsigned char, 1024ul>::SetLengthAndZeroTerminate(unsigned long) in util.o node::MaybeStackBuffer<unsigned char, 1024ul>::AllocateSufficientStorage(unsigned long) in util.o node::MaybeStackBuffer<unsigned short, 1024ul>::SetLength(unsigned long) in util.o ... ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: *** [/Volumes/code/forks/WORK-node/out/Release/cctest] Error 1 make[1]: *** Waiting for unfinished jobs.... rm fe2fa96b1035b05b2fe602777c9605ade7b05863.intermediate make: *** [node] Error 2 

Marking dont-land for now. Feel free to backport if it is needed. Thanks!

@addaleax
Copy link
Member

I think this just depends on the backport in #12664

@addaleaxaddaleax added lib / src Issues and PRs related to general changes in the lib or src directory. and removed dont-land-on-v7.x labels May 1, 2017
@jasnelljasnell mentioned this pull request May 11, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

@danbev should this land on v6.x? It cherry-picks cleanly, but I'm not sure whether it needs to depend on the other commits in #12664, some of which are semver-minor or semver-major.

@danbevdanbev deleted the remove-gtest-macro-guard branch June 28, 2017 05:41
@MylesBorins
Copy link
Contributor

ping @danbev

danbev added a commit to danbev/node that referenced this pull request Aug 15, 2017
As indicated by the FIXME comment, this macro guard is no longer needed. PR-URL: nodejs#12638 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
As indicated by the FIXME comment, this macro guard is no longer needed. PR-URL: #12638 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
As indicated by the FIXME comment, this macro guard is no longer needed. PR-URL: #12638 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for freeto 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 / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@danbev@addaleax@evanlucas@gibfahn@MylesBorins@bnoordhuis@jasnell@cjihrig@nodejs-github-bot