-
Notifications
You must be signed in to change notification settings - Fork 6
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
add query to twimlFor, backgroundTrigger, and transitionOut #50
base: master
Are you sure you want to change the base?
Conversation
I'm gonna leave this open because I'm not totally opposed to it, at least as a stopgap, but I think you might run into some issues with this approach. I'd say try it out on the hotline repo and update the PR as needed; once you get the hotline migrated to this successfully and work out any kinks, then we can talk about merging it. Among the current issues I see:
|
@data-doge fyi, i moved some files around today—looking at the code through your eyes made me realize how badly defined the boundaries were between various subcomponents—so this pr will need some updating. almost none of the logic has changed, though, just which files are where has changed, so the updates should be very easy/mechanical. |
@data-doge just saw that the latest tweaks here add a Part of me is hesistant to add express-configuration logic to twilio-ivr at all; it seems out of scope. I think maybe a preferred solution would be to configure an external app, and then have that app use twilio-ivr? I.e.: // in user-land code, configure app
const app = express();
app.set('trust-proxy', true);
// add twilio-ivr
app.use(twilioIvr(states, config)); Does that feel too clunky to you? If so, I could also maybe see accepting an |
yeah, external app sounds like a great idea. feels less prescriptive. |
eyyy if this is something you'd like in twilio-ivr, lmk, and i can update your docs.
otherwise, feel free to close