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

Upgrade SVGO to v3 #275

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upgrade SVGO to v3 #275

wants to merge 2 commits into from

Conversation

paulcpederson
Copy link
Member

Noticed that the action that would release automatically is currently failing. This is due to a lack of package-lock.json being committed to the repo. I removed our ignore of this file, and installed again, but then ran into an error. It seems that the old version of SVGO we were using is referencing a now defunct depedency so the install will always fail.

To address this, I've upgraded to the lastest SVGO version. I've reimplemented our SVGO options to use the new API, so the plugins should be acting mostly the same. Though the build shows lots of icons being changed, if you look at the diffs it is really small stuff like single characters changing in the path data.

Would be good for all you the icon designers to look at this branch and verify that nothing looks out of place.

@paulcpederson
Copy link
Member Author

To see a diff of the code that changed rather than the entire icon set diff, see this commit: dcf7f42

@paulcpederson
Copy link
Member Author

/cc @geospatialem

Copy link
Contributor

@SkyeSeitz SkyeSeitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience on this review, Paul! I have looked over the set, including assets with multiple paths with opacities, and verified there are no unwanted visual changes to the icons. Appreciate this bug fix!

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.

2 participants