search

indiana-university / rivet-icons

Icons for Indiana University's Rivet Design System
https://rivet.iu.edu/icons-stickers/
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

Update dependencies #92

Closed basham closed 2 years ago

basham commented 2 years ago

A dev alerted me to some dependabot alerts, resulting from xmldom and yargs-parser, which are dependencies of svg-sprite@1.5.0. Newer patch releases seems to have fixed some of these problems, over the last six months. This PR is meant to upgrade dependencies to resolve this issue. And I've upgraded other dependencies, too, since I was doing the work.

I also ran npm audit fix, to do some additional clean up in package-lock.json. I did not do npm audit fix --force, as that could cause breaking changes throughout the dependency tree.

As an aside, we should also update the readme's Quick Start code to reflect the latest version. We didn't do this with the v1.0.1 release.

For example, this URL was never updated:

https://unpkg.com/rivet-icons@1.0.0/dist/rivet-icons.css

We could update it so it works for any major v1 releases:

https://unpkg.com/rivet-icons@1/dist/rivet-icons.css

Or, we could just make it resolve to the latest release:

https://unpkg.com/rivet-icons/dist/rivet-icons.css

What should we do here?

Once this work is merged, would you mind doing a v1.0.2 release, @levimcg? Thanks!

basham commented 2 years ago

After the dependency upgrades, running npm run build does not change any of the generated files. So, that's comforting.

And the docs site seems to run fine, too, with npm run start.

levimcg commented 2 years ago

@basham I'll make a note to update the links to latest dependencies when I do the release. I'll open a PR and tag you for a review on that.

We could update it so it works for any major v1 releases:

I like this approach best, but would also be fine just pointing at the exact version number.

basham commented 2 years ago

I like the major v1 option the best, too. It inherently says that this code sample will work for that major version. And it means we don't need to update the readme every time there's a minor or patch release. That reduces some admin overhead and potential mishaps.

levimcg commented 2 years ago

Cool, lets plan on going the major version v1 option. I'll go ahead and merge this PR.