Replace Restify with Express #80
Conversation
|
|
||
| fs.readdirSync('./middleware').map(name => { | ||
| require('./middleware/'+name)(app); | ||
| }); |
There was a problem hiding this comment.
How do middlewares get loaded if this function is removed?
There was a problem hiding this comment.
Yeah, I removed as this as after the cors and jwt middlewares were removed and now simply done with app.use, I figure if we - as authors - are adding anymore it'd be the same way; either application level middleware, or routing level middleware. Because there are no middleware generators, then there is no need to read and load them dynamically.
If a future redbeard user wants to add middleware, sure, they can add a separate folder for it if they so wish.
There was a problem hiding this comment.
I think it's worth having an opinion about where middleware should go. It saves people having to re-make that decision every time. They're still welcome to make a different decision, because it's just code that they can edit. If we give a sane default, though, they're not forced to.
Also, while we are no longer adding middleware as part of base we're still likely to want redbeard middleware in other generators. User is the main area that springs to mind.
There was a problem hiding this comment.
Sure, if you think it's still useful, I'll add it back in.
|
In general, looks great! Nice work. I have two questions that I've asked in line comments. |
|
@davidbanham Does my response on middleware make sense? If yes, I'mma merge this sucker. |
|
Will merge as soon as this passes CI. |
This branch resolves #5, #64 and #73.
Restify is gone. Long live Express!
A few things:
express-jwtinstead the existing custom implementation. Test files have been repurposed appropriately./base.kiss-my-error.