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

use existing app.server #13

Merged
merged 1 commit into from
Feb 8, 2016
Merged

use existing app.server #13

merged 1 commit into from
Feb 8, 2016

Conversation

laggingreflex
Copy link
Contributor

Checks to see if app.server exists, if so, uses that, if not, creates one.

Wraps app.listen with warning only if app.server was created here.

Doesn't throw error if app.server exists (this is causing to fail 1 test in attach.test.js 'should not alter a koa app that already has .server')

@mattstyles
Copy link
Owner

Yeah, looks good to me. Can you fix up the test or remove it and I'll merge it in!

Thanks

@laggingreflex
Copy link
Contributor Author

Updated. It also checks now whether app.server if already exists is actually an http server. (app.server.constructor.name should be "Server"), just in case.

Updated tests.

Checks to see if `app.server` exists, if so, uses that, if not, creates one.

Wraps `app.listen` with warning *only* if `app.server` was created here.

If `app.server` exists but it's not an http server, throws an error.
@mattstyles
Copy link
Owner

Thats awesome!

Thanks!

mattstyles added a commit that referenced this pull request Feb 8, 2016
@mattstyles mattstyles merged commit 038d72e into mattstyles:master Feb 8, 2016
@laggingreflex
Copy link
Contributor Author

publish to npm please

@mattstyles
Copy link
Owner

Yep, all in 4.1.0. I was going to add some other stuff but that'll take longer so it's up now.

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.

None yet

2 participants