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

accept URL as well as domain name for --mirror argument #252

Merged
merged 2 commits into from Apr 7, 2020

Conversation

tchaikov
Copy link
Contributor

do not assume that the mirror mirrors the full directory structure of
nodejs.org, actually, some mirros just have the part of
https://nodejs.org/download/release/ mirrored. for instance,
https://npm.taobao.org/mirrors/node, and do not assume that the mirror
always put the nodejs mirror in its download/ directory.

in this change, a URL is accepted as well as a domain name. if a mirror
does not have "://" in it is specified, the default
"https://<domain_name>/download/releases" URL is used.

this change addresses the comment of
#208 (comment)

Signed-off-by: Kefu Chai tchaikov@gmail.com

@tchaikov
Copy link
Contributor Author

tested using

$ tox -e py37 # i added py37 in the envlist for testing in my env
$ venv/bin/pip uninstall nodeenv
$ venv/bin/pip install -e ~/dev/nodeenv
$ venv/bin/nodeenv --mirror=https://npm.taobao.org/mirrors/node -p --node=10.19.0
$ venv/bin/nodeenv -p --node=10.19.0

@tchaikov
Copy link
Contributor Author

tchaikov commented Mar 2, 2020

@ekalinin hi Eugene, anything i can do on my side to progress this change?

@ekalinin
Copy link
Owner

ekalinin commented Mar 2, 2020

Please, add:

  • an example into README: how to use new option
  • a test

@tchaikov
Copy link
Contributor Author

@ekalinin hi Eugene, updated and repushed. could you please take another look?

do not assume that the mirror mirrors the full directory structure of
nodejs.org, actually, some mirros just have the part of
`https://nodejs.org/download/release/` mirrored. for instance,
https://npm.taobao.org/mirrors/node, and do not assume that the mirror
always put the nodejs mirror in its `download/` directory.

in this change, a URL is accepted as well as a domain name. if a mirror
does not have "://" in it is specified, the default
"https://<domain_name>/download/releases" URL is used.

this change addresses the comment of
ekalinin#208 (comment)

also, ignore E127 for multi-line `with` statement, see discussion at
PyCQA/pycodestyle#316, which is related to
E126, but it also applies this case.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
node.js and io.js already merged back together since v4.0. and iojs.org
redirects to nodejs.org. so, it does not make sense to offer this option
for new developers using node.js >= 4.0

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@fredvd
Copy link

fredvd commented Mar 27, 2020

@ekalinin If you have some time available, could you please look again at this pull request and make a new release? The --mirror improvement would be very helpfull to run nodeenv behind firewall in restricted environments.

If a new release is made, would it be possible to also publish the source version again? (#251)

@tchaikov
Copy link
Contributor Author

tchaikov commented Apr 7, 2020

@ekalinin hi Eugene ping?

@ekalinin ekalinin merged commit 2ba54b4 into ekalinin:master Apr 7, 2020
@ekalinin
Copy link
Owner

ekalinin commented Apr 7, 2020

@tchaikov @fredvd Merged.

Sorry for delay, guys. A bit difficult situation at the moment, absolutely have no free time :(

@tchaikov tchaikov deleted the mirror-with-url branch April 7, 2020 07:59
@tchaikov
Copy link
Contributor Author

tchaikov commented Apr 7, 2020

thank you, @ekalinin . fully understand. take care.

@ekalinin
Copy link
Owner

ekalinin commented Apr 7, 2020

thanks @tchaikov! take care too

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

3 participants