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 of remote_path for tag is not ideal #18

Open
thom-nic opened this issue Jan 13, 2017 · 5 comments · May be fixed by #19
Open

Use of remote_path for tag is not ideal #18

thom-nic opened this issue Jan 13, 2017 · 5 comments · May be fixed by #19

Comments

@thom-nic
Copy link

Hi Bill -

I tried to submit a PR to the node-serialport project (see serialport/node-serialport#1055) to split the host and remote_path portions of the node-pre-gyp spec in package.json. This was so that I could easily override the host used by node-pre-gyp to create private pre-built binaries for native node modules that don't have pre-built support for my target arch (ARM.)

As you can see in the linked thread, we ran into trouble because node-pre-gyp-github wants most of the remote path to be in the host portion of the specifier. It's mostly an annoyance as I noted a workaround in this comment but wanted to open an issue here in case you think anything can be done to improve how node-pre-gyp-github works in this regard. Maybe use a binary.tag specifier that is prioritized when present over remote_path?

FWIW here are some examples of the canonical (and IMO more intuitive) use of the host and remote_path options:

I understand this is probably not your top priority so happy to submit a PR if you are open to possible improvements. Thanks!

@bchr02
Copy link
Owner

bchr02 commented Jan 13, 2017

I'm definitely open to improvements. Send me a pull request so I can have a look. I prefer this not be a breaking change that would require a MAJOR release though if possible (and practical). Thanks.

@thom-nic
Copy link
Author

yeah I'm thinking existing behavior would be preserved unless a new binary.tag attribute is present? And hopefully no existing users would have this already!

@bchr02
Copy link
Owner

bchr02 commented Jan 13, 2017

We can just make it a MAJOR release and put a section on the README about moving from one version to the other. No big deal. At least it's not going to require most people to rewrite anything unless the remote chance they are using binary.tag .

@thom-nic
Copy link
Author

👍 thanks I'll try to get a PR for this soon.

@bchr02
Copy link
Owner

bchr02 commented Jan 13, 2017

Sounds good. Thank you!

thom-nic added a commit to thom-nic/node-pre-gyp-github that referenced this issue Jan 16, 2017
@thom-nic thom-nic linked a pull request Jan 16, 2017 that will close this issue
thom-nic added a commit to thom-nic/node-pre-gyp-github that referenced this issue Jan 17, 2017
thom-nic added a commit to thom-nic/node-pre-gyp-github that referenced this issue Jan 17, 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 a pull request may close this issue.

2 participants