svgo icon indicating copy to clipboard operation
svgo copied to clipboard

Fix Bug #1946

Open codermapuche opened this issue 1 year ago • 3 comments

Optimize a large number of vectors sequentially instead of parallel to avoid the simultaneously opened file limit

codermapuche avatar Jan 28 '24 00:01 codermapuche

I don't like how this code is formatted. Just make it asynchronous if you want to use Promises, it makes it easier to read.

KTibow avatar Jan 28 '24 00:01 KTibow

@KTibow Feel free to format it however you prefer, I just tried using the tool, it broke and I fixed it, I hope this code helps you improve it faster

codermapuche avatar Jan 28 '24 00:01 codermapuche

I don't like how this code is formatted. Just make it asynchronous if you want to use Promises, it makes it easier to read.

Agreed, this is an odd way to approach it in JavaScript. I'm guessing it's just habits from other languages.

Feel free to format it however you prefer, I just tried using the tool, it broke and I fixed it

If you're not prepared to address feedback constructively, it's better to just open an issue instead of a pull request. Not to mention, there are literally failing tests. ^-^'


While this does technically resolve the problem, it's also problematic to drop the parallelization.

  • We should continue handling multiple files at once, either batching according to cores count, using an arbitrary number, or using a library like graceful-fs which handles it automatically via backoff.
  • If we do any solution other than use graceful-fs, we should still check for the file limit error and provide a helpful error message.

In any case, happy to address formatting later, but could you please resolve linting and tests? Feel free to read the CONTRIBUTING guide for more context.

SethFalco avatar Jan 29 '24 22:01 SethFalco