hexo-asset-pipeline
hexo-asset-pipeline copied to clipboard
refactoring & improvements
PR features
-
Performance issues
- Too many buffers conversions
- For loop overhead in comparisons and replace operations
- Prevent object mutations in loops
-
External plugins completable increased
-
Others bugs and issues
- Solved relative paths problem
- Solved unreplaced img refs in css files, when hexo images is behind of css files
- Code style improved
- ReadMe added new features
-
New features
- Keep original files without replacing assets refs to revisioning files
- Now you can replace assets refs in ignored files, but not filename of itself by
replace_in_ignoredopts - Relative paths is supported now and may be customized by
relative_dirsopts - Imagemin support excluded opts. For example you need to optimize only
css/images/**files
Hi,
I have a few feedback on your PR:
1/ Thank you
2/ I see that you made multiple independent changes. To ease the review of those changes, could you isolate them in separate PR? Also, you pushed the package-lock.json Although that is a good practice in general, please don't do it in this PR
Also, you have not pushed any unit test.
Maybe someone familiar with this plugin will want to review your PR "as is"? @hexojs/core ?
Already use in production that during past six months. Have no issues yet. Too many changes, and too many time is gone sorry can't make a separate pr's. Would be great if someone can review from hexo core team. About units, sure i can cover new features and remove lock json as well.
I will do a review ASAP.
Hey,
Can you remove package-lock.json and add some unit tests, it will make PR easy to read and understand.
I highly appreciate your contribution.
Added package-lock fix. And i tried to add some units, but i got strange errors in sandbox "ReferenceError: hexo is not defined" when process(ctx) is called (check my todos in css tests)
Hey @BusinessDuck,
I replaced hexo-asset-pipeline with BusinessDuck:master in my setup(that was working fine before) and got following error.
TypeError: Cannot read property 'length' of undefined
at checkFileIgnore (/Users/bmelkani/Projects/github/hexojs/tmp/hexo-asset-pipeline/lib/utils.js:43:36)
at /Users/bmelkani/Projects/github/hexojs/tmp/hexo-asset-pipeline/lib/filters/image.js:58:12
at Array.filter (<anonymous>)
at Hexo.run (/Users/bmelkani/Projects/github/hexojs/tmp/hexo-asset-pipeline/lib/filters/image.js:55:31)
...
To me changes in your PR looks, like breaking changes (which might also have some bugs or miss some corner cases). This will break working setups for users.
Since the change suggested by you is mostly code refactoring, sorry I am afraid that I would not be able to merge these changes.
I hope you understand this. Thanks for your time. If you can provide some small PRs then we can incrementally incorporate them in the plugin.
Thanks :)
no problem, i'll try to make separate pr's later. Whats about my question regarding with hexo sandbox?