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

#230 gearman_wait don't ignore signal by flag #231

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

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