Changes to become v1.x #30
Conversation
…y of the engine it modifies
This exposes the (potential) express 5 style asynchronous locate, and by blessing the view.locate() function, engines can use this to look up resources.
since i18n for a dust engine makes no sense as Kraken uses it, this test can be much simpler.
| } else if ((!err && stat.isDirectory()) || (err && err.code === 'ENOENT')) { | ||
| next(); | ||
| } else { | ||
| cb(err); |
There was a problem hiding this comment.
I guess it looks like there is no need to check that err is defined here..
There was a problem hiding this comment.
Yeah. I'm not super happy with that logic -- it works, it's correct, but it's not obvious.
| To run lint | ||
| $ npm run-script lint | ||
| ```shell | ||
| npm run cover |
There was a problem hiding this comment.
I had to run npm run-script cover. Is this due to my npm version? (2.5.1)
There was a problem hiding this comment.
No, run has always been an alias for run-script. What happened when you did npm run cover?
There was a problem hiding this comment.
oh. sorry. i was simply using npm cover and then straight to npm run-script cover.. I didn't properly read the doc above and try npm run. That works for me now that I've tried it.
| @@ -2,3 +2,6 @@ | |||
| node_modules/ | |||
| coverage/ | |||
| npm-debug.log | |||
| *.swp | |||
| test/fixtures/tmp/ | |||
| nyc_output/ | |||
There was a problem hiding this comment.
I think this needs to be .nyc_output/
|
Is there a sample express app anywhere currently using the v1.x pre-release version of engine-munger? |
|
Working on integrating it into a kraken app right now. |
|
Also, the tests fire up a complete app. |
|
Ah right. I guess the root of my question was really "which versions of adaro and other things go along with this?" but it's really right there in package.json already! Aside from the picky stuff I already noted I don't have any other concerns. But let me digest this some more to make sure I can learn as much as possible before giving a thumbs up. |
|
awesome. It's really the design and redrawing of module boundaries I want to make sure make sense. |
|
In terms of structure, what do you think about externalizing the edit: Let me restate this: would there be sufficient value to include the express view patch verbatim in this module, and then externalize all the "non-patch" stuff (like bcp47/i18n resolution)? That could make more clear how we will eventually utilize express once 5.x is released. |
| freshy = require('freshy'); | ||
|
|
||
| var test = require('tap').test; | ||
| var freshy = require('freshy'); |
|
Mostly the prototype creation is inside None of that is performance critical, since a new view class is created once per app in normal use, so having it be properly private by scoping made me happy. |
|
Ok. Officially in love with this PR. 👍 |
|
👍 |
Changes to become v1.x
No description provided.