Adar Dembo has posted comments on this change.

Change subject: thirdparty: Link curl against el6 workaround openssl
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5603/2/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS2, Line 506: even though curl is mostly written in C
Was that surprising because CPPFLAGS sounds like it means "C plus plus flags"? 
It's actually not; CPPFLAGS means "C preprocessor flags", and it's typical for 
autotools to consume include directory customizations via CPPFLAGS (I forgot 
about this when I suggested CFLAGS; sorry about that).

If that's what you meant, could you remove the comment, since autotools are 
behaving as expected?

Or if you meant "I was expecting to pass the include directory via CFLAGS", 
then just rewrite the comment as "Note: curl shows a message asking for 
CPPFLAGS to be used for include directories, not CFLAGS", to avoid misdirecting 
the reader into  thinking this is somehow to do with the programming language 
that curl was written in (C vs. C++).


-- 
To view, visit http://gerrit.cloudera.org:8080/5603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I19e6be946853a80dad0f496839212887e76db06f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to