Skip to content

Add zmq_timers support #657

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

Merged
merged 2 commits into from
Apr 23, 2025
Merged

Add zmq_timers support #657

merged 2 commits into from
Apr 23, 2025

Conversation

cacharle
Copy link
Contributor

Fixes #575

This is a draft to add support for the zmq_timers_* functions.

Please let me know what are the requirements to get it merged.

@cacharle cacharle marked this pull request as ready for review April 17, 2025 18:30
Copy link
Member

@gummif gummif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. What is missing is some minor fixes and some simple tests should be sufficient.

@cacharle
Copy link
Contributor Author

I've had to rename zmq::timers_t to zmq::timers, the first one was making the tests crash (I'm guessing a linker name conflict with libzmq)

Do you have any workaround to keep the _t convention?

@gummif
Copy link
Member

gummif commented Apr 18, 2025

I think zmq::timers is fine. Not a fan of the _t convention, also unfortunate we are using the same namespace as libzmq.

@cacharle
Copy link
Contributor Author

Alright then I think this PR is ready

@gummif
Copy link
Member

gummif commented Apr 23, 2025

Could you try rebasing your branch on latest master, I just fixed the CI runners.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14624243775

Details

  • 33 of 39 (84.62%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 86.301%

Changes Missing Coverage Covered Lines Changed/Added Lines %
zmq.hpp 33 39 84.62%
Totals Coverage Status
Change from base Build 14615581445: -0.07%
Covered Lines: 882
Relevant Lines: 1022

💛 - Coveralls

@gummif
Copy link
Member

gummif commented Apr 23, 2025

Very good. Thanks for this.

@gummif gummif merged commit 34a9b3a into zeromq:master Apr 23, 2025
9 of 10 checks passed
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.

Support for zmq timers?
3 participants