Skip to content

Conversation

@mrleblanc101
Copy link

@mrleblanc101 mrleblanc101 commented Jul 7, 2022

Fix #60

@mrleblanc101 mrleblanc101 marked this pull request as ready for review July 7, 2022 21:33
@mrleblanc101
Copy link
Author

Tested with npm link and work great 👍

@jpkleemans
Copy link
Owner

Hi Sébastien, thanks for your PR! I'll look into it next week.

@mrleblanc101
Copy link
Author

Thanks

Copy link
Owner

@jpkleemans jpkleemans 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 work. Could you review my comments?
Also, is it possible for you to add a cypress test for this change?

@mrleblanc101
Copy link
Author

mrleblanc101 commented Jul 15, 2022

Actually I think path should be passed to svgo.optimize({ path: 'path.svg' }) alone and the options should be passed to new svgo(options) but you don't seem to instantiate the plugin like that.

@mrleblanc101
Copy link
Author

mrleblanc101 commented Jul 15, 2022

Sébastien LeBlanc added 2 commits July 18, 2022 16:00
@mrleblanc101
Copy link
Author

Thanks for your work. Could you review my comments? Also, is it possible for you to add a cypress test for this change?

I added one test

@jpkleemans
Copy link
Owner

Actually I think path should be passed to svgo.optimize({ path: 'path.svg' }) alone and the options should be passed to new svgo(options) but you don't seem to instantiate the plugin like that.

In the svgo readme, it looks like you can add config to optimize just fine: https://github.com/svg/svgo#optimize

@jpkleemans
Copy link
Owner

Thanks for adding the test!
Can you fix the merge conflicts? Then I can merge the PR

@mrleblanc101
Copy link
Author

@jpkleemans I fixed the merge conflicts.

@jpkleemans jpkleemans merged commit ce3e75e into jpkleemans:main Sep 1, 2022
@jpkleemans
Copy link
Owner

Thank you!

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.

svgo.optimize missing path option

2 participants