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

include windows binaries #50

Closed
wants to merge 4 commits into from
Closed

include windows binaries #50

wants to merge 4 commits into from

Conversation

soliton4
Copy link

this is my very first pull request ever so pls dont be mad if i screwed it up.

this is basically what i have done in node-mirror. i did not actually test my modifications. just wanted to show you how i would approach the matter.

a actual test would have to go successfully through npm and since you are using some dev tools i am not familiar with i could not do a actual test.
but it works fine in node-mirror so it would be cool if we deliver windows binaries based on this proof of concept.

@TooTallNate
Copy link
Collaborator

I appreciate the effort but like I said in #45, I don't want to adopt any binary inclusion strategy until node.js core has some blessed solution for this. We'll just have to wait for now.

In particular, this solution seems to bundle things like mocha directly into the repo which, which I can't really see a reason to do and is really dirty in the diff.

Thanks, but no thanks.

@soliton4
Copy link
Author

hold on a second here.

its true that my approach was to simply throw everything from the build version into a directory. which is totally messy and includes a lot of unneccessary files.
this does not mean however that you would have to do so. you or at least peters knows which files are neccessary and can add only those files. which makes the diff much much cleaner.

i have borrowed this solution from socket.io which seems to do exactly that when no local build environment is present on a windows pc. also there does not seem to be any blessed node.js solution for including binaries. and i can see why. if i would be in charge of designing the npm api i would say "why bother? if you wanna include binaries just do so" which seems to be exactly what other projects are doing.

ddm pushed a commit to ddm/pty.js that referenced this pull request Nov 8, 2017
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.

2 participants