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

Updates restify peerDependency #6

Merged
merged 1 commit into from
May 11, 2015

Conversation

mehdivk
Copy link
Contributor

@mehdivk mehdivk commented May 6, 2015

Adding support of Restify 3.0.x

@mehdivk
Copy link
Contributor Author

mehdivk commented May 7, 2015

Current tests are green.

@nguyenchr
Copy link
Contributor

@mehdivk I don't remember if there is a reason we didn't include Restify 3.0.x
@rprieto can you think of any?

also can you add a .travis.yml so we can get the build status

@mehdivk mehdivk force-pushed the restify-dependency branch from 77d447f to 27762f5 Compare May 8, 2015 01:12
@mehdivk
Copy link
Contributor Author

mehdivk commented May 8, 2015

@nguyenchr .travis.yml came via another PR. I can see The Travis CI build passed in Merge pull request area

@rprieto
Copy link
Contributor

rprieto commented May 8, 2015

Nice one 👍 I don't think there was a particular reason apart from the fact v3 didn't exist yet.

Probably just worth testing locally, in case the CORS implementation has changed in Restify (since this module implements preflight, but relies on Restify for actual requests).

@wlkrw
Copy link

wlkrw commented May 8, 2015

+1 Before I noticed this pr I was going to make one for relaxing the peer dependency. I tested with restify 3.0.2 for work, it's not on Github. How 'bout them Waratahs

@mehdivk
Copy link
Contributor Author

mehdivk commented May 11, 2015

@rprieto @nguyenchr @wlkrw
I tried it with Restify 3.0.2 and it works. Moreover last update on cors.js file of Restify codebase was 10 months ago and CORS feature of Restify doesn't depend on other dependencies so I think it's safe to upgrade it.

https://github.com/mcavage/node-restify/blob/master/lib/plugins/cors.js

@rprieto
Copy link
Contributor

rprieto commented May 11, 2015

Awesome, thanks Mehdi! Let's merge this 🚢

rprieto added a commit that referenced this pull request May 11, 2015
@rprieto rprieto merged commit 758fbfc into Tabcorp:master May 11, 2015
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.

4 participants