Closed jimbol closed 11 years ago
It looks like you have a few duplicate commits, I already fixed the license and the compiler is now RC 6.1 friendly (using handlebars 1.0.0). Could you pull out the files that are not middleware related so I can clearly see the diff (keep in mind, I did a force push earlier in the week after I merged in your changes, this might be causing the issue. If so, could you do a pull of master first and get to a clean working state?)
Sorry for the re-work, but the merge earlier in the week was extremely broken
Its cool, I'm a bit new to working with git. I'll see what I can do to clean this up. Thanks for your patience.
So I've gotten latest from your repository, the only changed files should be main.js
, package.json
, and README.md
. There is also one added file, middleware.js
.
Is this clean enough?
Jim
I don't want to sound impolite, but I think the middleware component you added would be better as a stand alone npm module that simply uses this one (instead of adding more behavior to this npm module itself).
One of the philosophies of node.js that I've adopted is to create a lot of very small modules that do one thing well.
An example => If ember.js eventually changes the way you compile handlebars templates I would prefer to modify only one module (the one that does the compile step). That way your stand alone module could just bump the required version of this when that changes (if needed) -instead of having to change the core middleware module itself.
It seems like the middleware module has reason enough to stand on its own (even if you think the behavior is very small in itself, it's doing something independent of this compiler).
I really don't want to come off ungrateful or negative here as you put in a great deal of work (and from one developer to another, I really appreciate it because it was your time).
No problem, I just figured it was an extension of sorts. I'll create a new module. Thanks anyway!
I've added the ability to run as middleware. Updated the README accordingly.