-
Notifications
You must be signed in to change notification settings - Fork 2
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
[WIP] Allow setting barge per-prompt #18
base: develop
Are you sure you want to change the base?
Conversation
@@ -119,14 +127,18 @@ def on_failure(&block) | |||
|
|||
def run | |||
@errors = 0 | |||
deliver_prompt | |||
deliver_prompt true |
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.
Following up on CS discussion we had last week: what do you think about converting this to a hash so it reads more naturally?
deliver_prompt interruptible: true
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.
Happy to do so using keyword arguments, meaning 2.0.0+. What do you mean "CS discussion we had last week"?
b9e16d2
to
f357c79
Compare
self.class.barge | ||
else | ||
old = @barge | ||
@barge = nil |
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.
Why not move this to a separate method, something like reset_barge
, even if it's private?
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.
Also, if I'm reading this correctly, this method isn't idempotent. Reading the value changes it, which feels dangerous.
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.
Yep, probably needs refactoring.
3347921
to
d2dff32
Compare
Can I get a CHANGELOG note and a README update? The intent of the previous commit was to prevent prompt bouncing. Perhaps we can be more intelligent by default, and switch off bargeability if the previous prompt played for shorter than some cutoff. That way we don't penalize people who are using the barge without a problem. Another thing I noticed is that it doesn't appear to be possible to turn off speech-barge while leaving dtmf-barge enabled. Is there a way around that? Not being able to barge with DTMF definitely breaks expectations, even if speech barge is disabled. |
No description provided.