search

himynameisdave / postcss-plugins

The "officially unofficial" consolidated list of PostCSS plugins in a ready-to-use package
MIT License
127 stars 153 forks source link

Added plugins to list #288

Closed gavinmcfarland closed 4 years ago

gavinmcfarland commented 4 years ago

Hi, I've added some of my plugins to the list. I encountered a few things though, when installing the dependencies it creates a package-lock.json, should this be in the repo? I've omitted it for this commit. Also when running the script it didn't update the stars so I've manually updated them both in plugins.json and docs/authors.md.

gavinmcfarland commented 4 years ago

Hi,

Thanks for the feedback. Just to let you know I didn't edit the plugins.json or authors.md file directly. There was a linting issue so I copied them across (after adding them using the script) to a new fork which you said was fine https://github.com/himynameisdave/postcss-plugins/issues/286. You'll see the person's plugin has been overridden in both plugins.json and authors.md, I wouldn't have done this by accident twice, so I'm confident it was the script that did this.

I read the steps, but can I suggest maybe adding a note about not having to update the stars (and that it will happen when you run before a new release) because as an I contributor I need to check if my commits are correct, but if the stars are not correct in plugins.json then I won't know they will be updated by you.

Also, would it be a good idea to add package-lock.jsonto the .gitignore file, if you don't want it to be committed? Because it's one more thing for contributors to have to question/worry about when committing changes.

I'll redo it by adding my plugins again using the script and see how it goes.

himynameisdave commented 4 years ago

@limitlessloop great idea about putting package-lock.json into .gitignore, I hadn't thought about it. 👍 I've also updated the steps in the README to indicate that users don't need to worry about stars 👍

I wouldn't have done this by accident twice, so I'm confident it was the script that did this.

That sounds correct, and would love to help debug this some more. Do you happen to remember the exact steps you did to get into a state where the previous entry was overridden?

gavinmcfarland commented 4 years ago

I believe I installed the dependencies and then followed the instructions on the script and copied the files across to a new fork (after having issues with linting). I tried to recreate it a second time around but I didn't encounter the same issue. I must have done something really weird when I first forked the repo. I'll try investigating a bit more. Meanwhile, I've submitted a pull request.