-
Notifications
You must be signed in to change notification settings - Fork 16
refactoring & improvements #43
base: master
Are you sure you want to change the base?
Conversation
Hi, I have a few feedback on your PR: 1/ Thank you 2/ 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. |
I will do a review ASAP. |
Hey, Can you remove 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 |
Hey @BusinessDuck, I replaced hexo-asset-pipeline with
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? |
PR features
Performance issues
External plugins completable increased
Others bugs and issues
New features
replace_in_ignored
optsrelative_dirs
optscss/images/**
files