search

malchata / rollup-plugin-imagemin

Imagemin meets Rollup!
25 stars 7 forks source link

feat: Merge plugin factory's settings with default, allow partial overrides of plugins #1

Closed GerkinDev closed 5 years ago

GerkinDev commented 5 years ago

Give to the user the ability to specify options to merge with default before passing them to plugin's factories. Allow to merge custom plugins with default plugins Add dynamic plugins configuration depending on the plugin's name

✔ Tests ✔ Docs ✔ Lint

I've read the Contributing guidelines, I'm aware I should have opened an issue about this feature, but I personnaly needed it, so I'm using my repository instead of your module. I just propose you to integrate it in your module if you'd like to.

malchata commented 5 years ago

Hi, I appreciate you doing this! I'm open to merging this, I just need to see what its effects are before I merge it in.

The contributing guidelines are there to largely get people to pause and think about why they're contributing something. In this case, I think there's a need for this. I just need to set aside time to do a proper review prior to merge.

Cheers, -j

GerkinDev commented 5 years ago

Hi there,

Do you need anything more to merge this PR ? Explanations, further docs, tests, etc etc... ?

malchata commented 5 years ago

I'll get an eye on it this evening. I'm sorry for the delay, I've been juggling a rather large TODO list.

GerkinDev commented 5 years ago

Yeah yeah no problem no hurry, I just know too much that it's easy to forget things, drowned by the stack of work to do ^^

malchata commented 5 years ago

This looks/works good. I'm going to pull this in and do a few little tweaks, then publish the latest.

GerkinDev commented 5 years ago

You rock man, thank you for the merge !

I've seen that you've re-linted most of the files I edited. Maybe you should add lint configuration files with the repo, so each contributor could auto-lint the repo using your config.

Cheers buddy :) 🍻

malchata commented 5 years ago

No worries. :)

I assume you mean something like Prettier? I do have eslint installed, and I don't think any of your changes introduced linter errors. It was more just little styling things. :)