Skip to content

Conversation

@borro
Copy link

@borro borro commented Mar 22, 2019

interrupt in gearman_wait function after signal arrive

@esabol
Copy link
Member

esabol commented Mar 22, 2019

Looks promising!

How about a test case?

@borro
Copy link
Author

borro commented Mar 22, 2019

How about a test case?

I am not a C++ programmer. I just made corrections in the code in an analogy, so that they look like yours. Writing unit tests in C++ is already above my knowledge.

@SpamapS
Copy link
Member

SpamapS commented Apr 2, 2019

@borro thanks, I understand where you're at. We may need to follow-up with a test case before we land this.

A test case could be tricky though, as we'll need to add signals to the test suite, which I'm not sure are in there, or we need to add an integration test that spins up gearmand and ensures it handles signals that way.

For now, does anyone want to try and figure out how to test this bit in the regular test suite?

@SpamapS
Copy link
Member

SpamapS commented Feb 18, 2020

Bumping this a bit. Seems useful for usre, but I'd still like to see a test. However, it seems nobody has found the time to figure that out. This adds an API call, so I wonder if we should consider bumping to 1.2.0 if we merge.

@SpamapS SpamapS marked this pull request as draft November 18, 2023 16:18
@SpamapS
Copy link
Member

SpamapS commented Nov 18, 2023

Hi @borro . Could you please file a separate issue that clearly explains the need for this? I converted it to a Draft PR for now ,so we're not confused by the fact that it's not really ready.

Copy link
Member

@SpamapS SpamapS left a comment

Choose a reason for hiding this comment

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

Needs better descriptions and a test.

@borro
Copy link
Author

borro commented Nov 20, 2023

@SpamapS all descriptions in Issue #230.
I repeat that I am not a C++ developer and have no idea how to write unit tests for C++ and signal processing.

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.

3 participants