Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pirates for require hooks. #5

Closed
wants to merge 2 commits into from

Conversation

ariporad
Copy link

@ariporad ariporad commented Dec 6, 2015

Hi!

As discussed in various issues (mainly istanbuljs/nyc#70), this PR uses pirates to implement require hooks. Pirates takes care of hooking require, so we don't have to. This makes the various require-related functions in this module mostly wrappers, but useful none the less.

This change is completely transparent (a patch version bump), and all the tests still pass.

@ariporad
Copy link
Author

ariporad commented Dec 8, 2015

@gotwarlost: Ok, Breaking API changes have been dealt with! Pirates@2.0.0 has been published, and this PR has been updated! I hope you enjoy!

@ariporad
Copy link
Author

@gotwarlost: Would it be possible to get this merged? If not, is there anything I could do to fix it? Thanks!

@ariporad
Copy link
Author

@gotwarlost: bump.

Ari Porad added 2 commits December 13, 2015 13:51
Pirates is a module which takes care of hooking require for you,
allowing you to just pass in a transformer.
@ariporad
Copy link
Author

(This might not be the right place for this, but...)

Hey, @davglass, you work with @gotwarlost in real life, right? Do you have any idea where he might be? He appears to have disappeared. Thanks!

@gotwarlost
Copy link
Contributor

Just on vacation for the past few weeks. Back now :)

Will look at istanbul related stuff next week since I have a few other things to finish before then

@ariporad
Copy link
Author

ariporad commented Jan 1, 2016

@gotwarlost: Ok, Great! Thanks!

@ariporad
Copy link
Author

@gotwarlost: bump.

@gotwarlost
Copy link
Contributor

Sorry man, @jamestalmage 's writeup https://www.npmjs.com/package/append-transform convinced me that I need it too. The lack of 'append' is causing many bug reports even on the istanbul 1.x branch.

Closing this PR as a result. Sorry!

@gotwarlost gotwarlost closed this Jan 25, 2016
@ariporad
Copy link
Author

Hey, no worries! Append-transform respects the chain, which is all I care about.

Thanks! Ari
On Sun, Jan 24, 2016 at 11:40 PM, istanbuljs/istanbul-lib-hook
reply@reply.github.com
wrote:
Closed #5 [https://github.com//pull/5] .


Reply to this email directly or view it on GitHub
[https://github.com//pull/5#event-524654080] .[https://github.com/notifications/beacon/ABu7pF-oqRCbphOgLshNvrRU3u4qzC5Eks5pdcjYgaJpZM4Gvk-T.gif]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants