alexa-js / alexa-app-server

An Alexa app server for alexa-app.
MIT License
401 stars 116 forks source link

Remove router from `app.express()` configuration #79

Closed rickwargo closed 7 years ago

rickwargo commented 7 years ago

alexa-app now deprecates simultaneous use of both expressApp and router in app.express() and generates the following warning:

Usage deprecated: Both 'expressApp' and 'router' are specified.
More details on https://github.com/alexa-js/alexa-app/blob/master/UPGRADING.md

Removing router option from config brings the app-server up-to-date with alexa-app.

dblock commented 7 years ago

Build looks broken. Needs an update for the dependency version?

rickwargo commented 7 years ago

Oops. That's what I get for the "simple, quick fix." This depends on the latest, non-released version of alexa-app. What to put for the dependency?

dblock commented 7 years ago

Looks like you released 3.2.0 so we're in business ;)

rickwargo commented 7 years ago

3.2.0 was not released - at least not by me. I've been waiting...

dblock commented 7 years ago

Oh you and @ajcrites both have red icons and I've been staring at a computer for far too long :) https://github.com/alexa-js/alexa-app/issues/189

rickwargo commented 7 years ago

No worries, @dblock. Need to look into this merge more as the tests have different results on my computer.

rickwargo commented 7 years ago

@dblock I finally got the alexa-app version update to get the tests to pass.

dblock commented 7 years ago

Merged. I haven't looked at that failing test that we xit-ed, what's up with it?

rickwargo commented 7 years ago

It fails on older versions of node, but passes on the last two. It's an odd test using an OOB port number to ensure it is caught. Haven't looked at the code, but perhaps we either check bounds on the port number and fail if OOB or remove the test.

dblock commented 7 years ago

Oh yes I remember. I guess we can leave it as is.

rickwargo commented 7 years ago

@dblock can we release a new alexa-app-server? This and alexa-app are out of sync with respect to the alexa-app deprecation and this PR brings it back to lockstep.

dblock commented 7 years ago

Want to help out and make a release? What's your npm e-mail?

rickwargo commented 7 years ago

@dblock sure. you can use npm at e.p.i.c.m.i.n.d.s..c.o.m (removing superfluous periods)

dblock commented 7 years ago

Sorry it wants an npmjs.com user, my bad.

rickwargo commented 7 years ago

You can use rickwargo as the username

dblock commented 7 years ago

@rickwargo Cool. Follow https://github.com/alexa-js/alexa-app-server/blob/master/RELEASING.md to make a release please.

rickwargo commented 7 years ago

@dblock I missed changing the README to reflect "for the stable release" from "the next release". Should I push a new release?

rickwargo commented 7 years ago

@dblock and left an empty (Next) section in the Changelog. Should I make these corrects, update the RELEASING doc, and push to 3.0.2? Also, what about changes to the RELEASING doc for the next release?

dblock commented 7 years ago

No need to push another release for a missed README, it's fine.

Make sure RELEASING is accurate. We don't change version numbers in it, it's just an example.

rickwargo commented 7 years ago

Haven't changed RELEASING (yet) - I could add more in RELEASING to make what I missed more clear. Since the version numbers were appropriate for this new version, I assumed they were in sync.

dblock commented 7 years ago

They get quickly out of sync :)

dblock commented 7 years ago

@rickwargo I see the release on NPM, but no updates on master for the READMEs and what not, forgot to push them?

rickwargo commented 7 years ago

@dblock wanted to get word back on what to do - then had a 3hr meeting that just finished. will do now. I'll leave RELEASING as-is.

rickwargo commented 7 years ago

@dblock I don't appear to have permission to push the changes to README/CHANGELOG/package.json.

dblock commented 7 years ago

You were sent an invite yesterday :)

dblock commented 7 years ago

https://github.com/alexa-js/alexa-app-server/invitations

rickwargo commented 7 years ago

Missed that entirely, @dblock. Thanks. Pushed to repo.