Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Shouldn't delete node_modules #108

Closed
burner1024 opened this issue Jan 22, 2023 · 8 comments
Closed

[BUG] Shouldn't delete node_modules #108

burner1024 opened this issue Jan 22, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@burner1024
Copy link

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?

  • Version 4.2
@burner1024 burner1024 added the bug Something isn't working label Jan 22, 2023
@creyD
Copy link
Owner

creyD commented Jan 23, 2023

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
Copy link
Author

burner1024 commented Jan 24, 2023

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
Copy link
Owner

creyD commented Jan 24, 2023

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
Copy link
Author

burner1024 commented Jan 24, 2023

Looks good.

@jorgenbs
Copy link

jorgenbs commented Mar 15, 2023

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
Copy link
Owner

creyD commented Mar 15, 2023

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

@creyD
Copy link
Owner

creyD commented Jun 30, 2023

@burner1024 Do you consider this bug fixed?

@burner1024
Copy link
Author

Yes, sure.

@creyD creyD closed this as completed Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants