search

gulpjs / rechoir

Prepare a node environment to require files with different extensions.
MIT License
48 stars 12 forks source link

wip #23

Closed tkellen closed 9 years ago

tkellen commented 9 years ago

cc @phated do you think this revised functionality should just be in interpret?

phated commented 9 years ago

@tkellen I definitely think this could go in interpret. it would allow for the tests to be co-located and it's just another method to expose, so interpret could continue being used as it is now but just expose the prepare method also. thoughts?

tkellen commented 9 years ago

My only hesitation is that we'd be shipping interpret with liftoff but expecting consumers to provide their own. That seems weird.

phated commented 9 years ago

I was thinking we could make it the default but they could override if desired

tkellen commented 9 years ago

Hrm. I think I'm gonna leave them separate. If someone wants the loading behavior but also wants to provide an entirely different interpret object, I don't think they should have to inherit all of the existing ones (as they would if this was bundled with interpret).

phated commented 9 years ago

That's fair. Want me to review/merge this? Any idea why travis is failing?

tkellen commented 9 years ago

Yeah, a review/merge would be fantastic. Haven't looked into why this is failing but it's all green on my end.

tkellen commented 9 years ago

looks like i messed up the changelog, force pushing a fix for that in a sec.

tkellen commented 9 years ago

ready for review

phated commented 9 years ago

hmm, toml test is failing for me

phated commented 9 years ago

It looks like the requires aren't being cleaned up in the tests. Let me dig in a bit

tkellen commented 9 years ago

bad variable scoping, perhaps? i just force pushed a fix. sorry, i should probably be doing fixups now.

phated commented 9 years ago

yeah, need to do fixups now. It seems you removed the afterEach cleanup block

tkellen commented 9 years ago

Shouldn't be needed, but I could be wrong. Thanks for looking.

phated commented 9 years ago

When toml is throwing, iced-coffee-script and babel loaders are showing up in the stack trace. not sure why things aren't being cleaned up.

phated commented 9 years ago

AHHA! Coffee-script seems to be hook Module.prototype.load in addition to require.extensions....ugh

tkellen commented 9 years ago

Shesh. Like every one of my modules for cli tools (interpret, v8flags, flagged-respawn, rechoir) cuts into the really gross underpinnings of node.

phated commented 9 years ago

Got almost everything sorted out but literate coffee/iced on the .md extension isn't working with my test alterations. If I let coffee-script hijack the load stuff works but I want to get each test running from a clean cache.

phated commented 9 years ago

got it. you are going to hate me :stuck_out_tongue:

tkellen commented 9 years ago

alright, alright, commit the code already. what abomination awaits? :p

phated commented 9 years ago

@tkellen reassigning to you for review of all my changes. Your stuff looks good, just need a sanity check on all the test stuff.

tkellen commented 9 years ago

Looking good! Will cut a release in a min.

tkellen commented 9 years ago

PS: thanks!