Skip to content

Conversation

@PeterDaveHello
Copy link
Contributor

@PeterDaveHello PeterDaveHello commented Feb 16, 2018

This will let curl automatically to send Accept-Encoding header to include the compression methods such like deflate, gzip, etc, to request compressed data from server, and automatically decompress the data if it's compressed, I believe that this is very like(pretty the same) what commit 0829a74 / PR #1083 did, will save the time and bandwidth on transferring data.

The output and other behavior should be the same as curl will automatically handle the process to add header and to decompress.

My first PR to this project, let me know if I need to revise/improve anything, thanks a lot!

@arcanis
Copy link
Member

arcanis commented Feb 20, 2018

Thanks! Technically this is correct, but I think we'll keep the command line as it is - I prefer to keep the one-liner to stay short and simple so that users can quickly parse it mentally and understand it. Sounds good?

@PeterDaveHello
Copy link
Contributor Author

What about to move all parameters in a variable? Though I think it's still very short here, it has obvious advantage, and don't think most of the users need to read this source code behind yarn, for those who do trace source code, if they don't recognize this parameter yet, it's also a good opportunity to learn that, as it's not a complex parameter, very easy to understand :)

@arcanis
Copy link
Member

arcanis commented Feb 20, 2018

As I see it there's only two cases:

  • Either the users will type the command themselves, in which case the perf gain is counterbalanced by the extra characters

  • Or the users will copy/paste the command, in which case they won't even notice the argument.

Not sure we gain a lot in either case.

@arcanis
Copy link
Member

arcanis commented Mar 24, 2018

I'm going to close this PR, but thanks anyway to have took the time to make this proposal! 🙂

@arcanis arcanis closed this Mar 24, 2018
@PeterDaveHello
Copy link
Contributor Author

hmmm, I guess most people will copy and paste it and in most cases the users don't need to do it manually? The changes are in the scripts/codes level only but not the user manual, so I'd help this is acceptable, thanks :)

@arcanis
Copy link
Member

arcanis commented Mar 25, 2018

Oh wait, I completely misread the PR, I remembered seeing the change in the readme but I must have been mistaken 😧

Yeah it totally makes sense, let's merge this!

@arcanis arcanis reopened this Mar 25, 2018
@arcanis arcanis merged commit aada10e into yarnpkg:master Mar 25, 2018
@PeterDaveHello
Copy link
Contributor Author

Thanks!

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