search

dchester / epilogue

Create flexible REST endpoints and controllers from Sequelize models in your Express app
846 stars 116 forks source link

Mapping `delete` => `del` for Restify is not now required #106

Closed o5 closed 8 years ago

o5 commented 8 years ago

I just tried epilogue with Restify v4.0.3 and got this error:

/project/node_modules/epilogue/lib/Controllers/base.js:83
  app[self.method](endpoint.string, function(req, res) {
                  ^

TypeError: app[self.method] is not a function

This pull resolved it.

mbroadst commented 8 years ago

@o5 hey, thanks for the contribution. Could you also update the package.json to update the version of restify used for testing? (This is probably why the CI run failed for your PR)

o5 commented 8 years ago

@mbroadst hmm, I don't know why :/

mbroadst commented 8 years ago

yeah it looks like (from the failed tests) you're not correct, that restify indeed still uses del instead of delete?

o5 commented 8 years ago

OMG! When I call restify.createServer() with some config, it ends with reported error.

You can try it, just change this line to:

app = server = restify.createServer({
      name: 'test'
})
mbroadst commented 8 years ago

@o5 wow, sounds like a bug for restify! Okay, if you modify the test to include an option on that line then it looks good. Please remember to file an issue with restify, I'm sure they wouldn't like that to slip through the cracks :smile:

o5 commented 8 years ago

You had right. Restify still uses del. Did you try it? :-)

mbroadst commented 8 years ago

Ah, sorry looks like the bug is indeed just with restify. Okay, I'll leave it to them then and close this out