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

remove logalot #121

Conversation

romainmenke
Copy link
Contributor

not counting dev dependencies

- 230 packages
+ 179 packages

logalot was only used in the postinstall script and did not add any real value.
Less dependencies means less surface for security issues.

https://www.npmjs.com/advisories/1753

Verified

This commit was signed with the committer’s verified signature.
romainmenke Romain Menke
@fuqua
Copy link

fuqua commented Oct 13, 2021

@1000ch @kevva

@romainmenke
Copy link
Contributor Author

Maybe useful to someone, we ended up removing everything from imagemin as a result of this issue. Too many times have we seen security alerts caused by libraries importing random logging utilities.

We are now just using https://github.com/GoogleChromeLabs/squoosh cli

@1000ch
Copy link
Contributor

1000ch commented Oct 14, 2021

@fuqua @romainmenke
Sorry for the late response,

I think we want to fix logalot itself ideally, but it should be fixed with native console at the moment, because logalot is not critical for the module feature at least. Let me fix imagemin modules in this way. cc: @sindresorhus @kevva

@romainmenke
Copy link
Contributor Author

@1000ch why not use console directly here?

@1000ch
Copy link
Contributor

1000ch commented Oct 14, 2021

Sorry, I'm not sure about the original intention... but guessing to provide better logging UI with better functionality.

@sindresorhus
Copy link
Contributor

I think just using console.log is fine for this purpose.

@1000ch 1000ch merged commit 3370a42 into imagemin:main Oct 14, 2021
@romainmenke romainmenke deleted the remove-logalot--sympathetic-red-wolf-e5ee418f60 branch October 14, 2021 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants