creyD / prettier_action

GitHub action for running prettier on your projects pull requests
MIT License
276 stars 87 forks source link

[BUG] Shouldn't delete node_modules #108

Closed burner1024 closed 1 year ago

burner1024 commented 1 year ago

What exactly happened?

  1. Run npm install
  2. Run action.

What should've happened? node_modules not deleted, as they are used in the following steps.

Where did you encounter the problem?

creyD commented 1 year ago

This happens because someone might not have the node_modules in gitignore. Will add an option "clean_node_folder" which defaults to True, but if you need it, you can disable it from the next version onwards.

burner1024 commented 1 year ago

I think node_modules normally should be in .prettierignore, and if they are not, there's probably a reason for that. That's 2 layers of protection already, and I don't really see the need for the 3rd one. (Or you could do git checkout -- node_modules, but still I think presuming what the users want do to is not a good policy).

creyD commented 1 year ago

I agree, just don't want to change functionality if someone uses it like that. Could you please check against prettier_action@dev to verify the new option is working?

burner1024 commented 1 year ago

Looks good.

jorgenbs commented 1 year ago

Had issues with this as well, took a some time to debug why my workflow was failing.

I agree, just don't want to change functionality if someone uses it like that.

Consider switching to semantic versioning for this package so its easier to introduce breaking changes like this. E.g. with release-please

creyD commented 1 year ago

@jorgenbs I thought what I was doing is semantic versioning. v4.3 introduces this change, see changelog, v4.2 was the previous version.

creyD commented 1 year ago

@burner1024 Do you consider this bug fixed?

burner1024 commented 1 year ago

Yes, sure.