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

Tool improvements #101

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

akx
Copy link

@akx akx commented Oct 31, 2019

  • don't invoke wget separately for each file
  • note that Cython is required for a successful install
  • pin the external download versions (submodules would be better still really)

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 31, 2019
@hoschwenk
Copy link
Contributor

Hello,
Sorry for the late reaction.
What would be the real advantage of calling wget with a list of arguments instead of a for loop on a list ? Is it slow for you ?
I feel that a list of tools which will be installed is more flexible and one can more directly see what is needed

@akx
Copy link
Author

akx commented Dec 4, 2019

What would be the real advantage of calling wget with a list of arguments instead of a for loop on a list ? Is it slow for you ?

As I mentioned in the commit message, wget can then reuse the same connection for all files and not do TCP dances and HTTPS negotiation again and again. It's plenty faster than the original.

I feel that a list of tools which will be installed is more flexible and one can more directly see what is needed

I'm not following what you mean here. There's no changes to the lists of files downloaded.

@akx
Copy link
Author

akx commented Feb 3, 2023

@hoschwenk Ping! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants