Skip to content
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

Remove ENABLE_DRAFTS option #565

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

stephanlachnit
Copy link
Contributor

This option added ZMQ_BUILD_DRAFT_API to the compile definitions. However, this should be propagated by libzmq (see zeromq/libzmq#4323 and #477).

Closes #477.

@coveralls
Copy link

coveralls commented Aug 2, 2022

Pull Request Test Coverage Report for Build 6957349874

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.6%) to 86.354%

Files with Coverage Reduction New Missed Lines %
zmq.hpp 6 82.88%
Totals Coverage Status
Change from base Build 6697216186: -0.6%
Covered Lines: 848
Relevant Lines: 982

💛 - Coveralls

@gummif
Copy link
Member

gummif commented Aug 2, 2022

This change will probably break something for someone using slightly older versions of libzmq.

@stephanlachnit
Copy link
Contributor Author

This change will probably break something for someone using slightly older versions of libzmq.

Yes, I would recommend merging this change only after a new release of libzmq (the commit in question is not part of a release yet), since it is fairly reasonable to expect people not to take a new version of cppzmq and an old version libzmq.

This option added ZMQ_BUILD_DRAFT_API to the compile definitions. However, this should be propagated by libzmq (see zeromq/libzmq#4323 and zeromq#477).

Signed-off-by: Stephan Lachnit <[email protected]>
@stephanlachnit stephanlachnit force-pushed the p-remove-draft-api-setting branch from 926f8c0 to 01bcc7d Compare November 20, 2023 23:27
@stephanlachnit
Copy link
Contributor Author

@gummif since libzmq v4.3.5 has been released with zeromq/libzmq#4194 included, this can merged now

@stephanlachnit
Copy link
Contributor Author

I also updated the CI to use 4.3.5. to actually use this change.

@gummif
Copy link
Member

gummif commented Dec 7, 2023

Sorry for the delay. I will see if I have time soon to do a review.

@stephanlachnit
Copy link
Contributor Author

@gummif friendly ping to check this :) This is quite a small change

@gummif
Copy link
Member

gummif commented Aug 12, 2024

OK I think this looks good. In the unlikely scenario someone has problem with enabling drafts (using older version of libzmq built with drafts and new version of cppzmq) they just need to add target_compile_definitions(${target} PUBLIC ZMQ_BUILD_DRAFT_API) to their cmake project.

@gummif gummif merged commit 6541dd1 into zeromq:master Aug 12, 2024
10 of 11 checks passed
@stephanlachnit stephanlachnit deleted the p-remove-draft-api-setting branch August 13, 2024 11:14
@stephanlachnit
Copy link
Contributor Author

@gummif any chance for a new release with this fix?

@gummif
Copy link
Member

gummif commented Nov 19, 2024

Yes, will try to do it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZMQ_BUILD_DRAFT_API is not added to compiler definitions for cppzmq target when ENABLE_DRAFTS is ON.
3 participants