Don't find websocketpp if not CPPREST_EXCLUDE_WEBSOCKETS. #1494
Conversation
Signed-off-by: Tao He <linzhu.ht@alibaba-inc.com>
|
Hi devs, any comment on this PR ? |
|
It's in my queue, it looks pretty good to me at first glance |
| ) | ||
| cpprest_find_websocketpp() | ||
| target_link_libraries(cpprest PRIVATE cpprestsdk_websocketpp_internal) | ||
| if(NOT CPPREST_EXCLUDE_WEBSOCKETS) |
There was a problem hiding this comment.
is CPPREST_EXCLUDE_WEBSOCKETS ever actually set anywhere. From my reading this will result in no actual change, since CPPREST_EXCLUDE_WEBSOCKETS will expand to "" (and cause a warning) or "false" and thus websockets will be found
I could be missing something.
There was a problem hiding this comment.
I suppose one could set it on the command line, but in that case it should really be an option
And given we already have CPPREST_WEBSOCKETS_IMPL I don't see what the point is.
There was a problem hiding this comment.
Thanks for pointing out the usage of CPPREST_WEBSOCKETS_IMPL and after reading the code about CPPREST_WEBSOCKETS_IMPL=none branch again I have realized that this PR is unncessary.
I will close this PR, and many thanks for your review!
|
how does this differ from just setting "CPPREST_WEBSOCKETS_IMPL" to "none"? |
2 participants
Otherwise cmake will requires cpprestsdk available even when we not need it (and have excluded it).