Closed mattmazzola closed 6 years ago
@ryanvolum Do you agree with this approach? Let me know if you want other changes.
Regarding the prefixes, I modeled this package off of the bot framework directline API.
I like the approach you've taken and at face value agree that it cleans things up. Since this is a relatively major change, I'll do some testing this week and let you know what I think. I'd also like to more deeply understand your need, as I think others likely need the same functionality. Would you be free to meet to discuss your need more specifically?
Yea, open to meet and discuss.
I explain a bit about use case / needs in the referenced issue here: https://github.com/ryanvolum/offline_dl/issues/27
At a bit more high level: Our current application had many different ports for the consumer to manage (Bot, Directline, our own SDK) and each of this ports could have had conflicts with existing running services on the developers machines so then we had to add configuration to allow developer to control each of these ports. Then we had also inherited the issue of directline service url possibly being out of synch with the actual port (due to mis undertsanding of library before I read it). Our goal was to remove all this configuration /complexity by consolidating down to a single express server on single port and merely add or remove different routes based on desired functionality. This reduces possible confusion and errors for our customers.
Aside from our use case. I think using the express router and giving the consumers control of server listening is aligning with best practices for server / express based libraries like this one.
@mattmazzola this makes sense and the updates check out. Just accepted, merged and published your changes to offline-directline@1.2.5
Thanks for contributing!
I think the prefix should be removed from these routers, but they're not consistent so I left them.
This could be further done be exposing 2 routers (1 for directline and 1 for v3) or 4 total (one for each endpoint) although I thought that would expose too much complexity.
Fixes #27