apache / cordova-fetch

Apache Cordova Fetch Library
https://cordova.apache.org/
Apache License 2.0
27 stars 27 forks source link

CB-14133 Avoid reinstalling already installed packages #24

Closed raphinesse closed 6 years ago

raphinesse commented 6 years ago

This is to alleviate the performance penalty incurred by fetching-by-default.

This speeds up cordova prepare in below setup by about 200% (i.e. it takes half the time)

git clone https://github.com/dpogue/cb-13797
npm i
cordova prepare

I'm sure there are other issues related to this, but I linked to the one that made me think of a robust and easy way to increase performance.

In the long run we should try to reduce all fetching to a minimum, but that has to be tackled in cordova-lib.

Future Work

raphinesse commented 6 years ago

You're too generous :bowing_man: Should we merge or let it sit for a while to wait for more reviews?

dpogue commented 6 years ago

It's probably easier for people to test if we merge and it gets into the nightly builds

raphinesse commented 6 years ago

Good point. I'd make this a real merge to keep diff readability. Are you fine with that?

dpogue commented 6 years ago

yep

kirillgroshkov commented 6 years ago

I'm just wondering - what are these binary files in the end of PR, e.g spec/support/repo-name-neq-plugin-id.git/refs/heads/master? Are they here by mistake or it's part of the plan?

raphinesse commented 6 years ago

@kirillgroshkov they are local test fixtures. To speed up a previously network-dependent test. See https://github.com/apache/cordova-fetch/commit/36ecb8c105dd6b2bc0c18f9abd30cc444150e8db

kirillgroshkov commented 6 years ago

@raphinesse Thanks for clarification:)