-
Notifications
You must be signed in to change notification settings - Fork 31
fix: upgrade svgo to latest version and make sure the comman uses recursive flag to traverse all files #3089
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
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 28a9f05. ☁️ Nx Cloud last updated this comment at |
0b461ee
to
e96219d
Compare
📬Published Alpha Packages:@codecademy/[email protected] |
🚀 Styleguide deploy preview ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@@ -1,6 +1,6 @@ | |||
# EditorConfig is awesome: http://EditorConfig.org | |||
|
|||
root = true; | |||
root = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to check — Is this intentional? Did a formatting script do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is intentional this was breaking for Neovim, lt looks like ; is not allowed on line ends
yarn.lock
Outdated
@@ -13821,7 +13821,7 @@ svg-parser@^2.0.2, svg-parser@^2.0.4: | |||
resolved "https://registry.npmjs.org/svg-parser/-/svg-parser-2.0.4.tgz#fdc2e29e13951736140b76cb122c8ee6630eb6b5" | |||
integrity sha512-e4hG1hRwoOdRb37cIMSgzNsxyzKfayW6VOflrwvR+/bzrkyxY/31WkbgnQpgtrNp1SdpJvpUAGTa/ZoiPNDuRQ== | |||
|
|||
svgo@^1.2.2, svgo@^1.3.2: | |||
svgo@^1.2.2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if this Is this still necessary? Is it a dependency of some other package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LinKCoding I think it's coming from svgr/cli, but looking at the repo I don't think if we are using it anymore, will it be safe to remove this package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for doing this! i took a look at removing svgr/cli locally and it seems fine to remove. will ✅ once i've looked at the monorepo with that package removed just to be sure.
…ursive flag to traverse all files
e96219d
to
28a9f05
Compare
Overview
We noticed that svgo was on a very old version and did not support the recursive(-r) flag and hence doing
yarn run format
was not doing anything. This PR updates the svgo version to the latest and removes the yml config in favor of the new config format.PR Checklist
Testing Instructions
We need to test the svgs are functioning correctly after this update
PR Links and Envs