Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
v1.11.0 Release #2018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v1.11.0 Release #2018
Changes from 18 commits
1eaede8
f107343
4f1d3ad
b54d180
c93c47c
323d91d
936f5ec
b2162c6
4cd8eed
6bc0d2d
4b5ed47
e40fab5
ad2922a
38e942c
3652e86
df7f38b
096cafe
e22e08b
d9ab52d
d5d09c8
77e2c47
86aed1d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target_link_libraries(kvsCommonLws + ${PRODUCER_CRYPTO_LIBRARY} ${OPENSSL_CRYPTO_LIBRARY} ${OPENSSL_SSL_LIBRARY} ${LIBWEBSOCKETS_LIBRARIES}
Looking at the cmake 1-line change in this Producer C branch, its seems we should only link the
${OPENSSL_CRYPTO_LIBRARY} ${OPENSSL_SSL_LIBRARY}
libraries if not opting for MbedTLS - or better yet only have thePRODUCER_CRYPTO_LIBRARY
one which should cover both cases:So I propose the following change:
And with a nit variable name change that makes more sense as there are multiple libraries and to be more aligned with how we also named
LIBWEBSOCKETS_LIBRARIES
:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I thought of this as well, I have no idea why it was set up the way it is, however the openssl cmake variables will be blank in this case so it will be ignored by the target link libraries (I verified this). Preference though would be to do as you suggested as the final change.