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

update favicons. closes #57 #59

Merged
merged 6 commits into from
Sep 21, 2021
Merged

Conversation

miguelcobain
Copy link
Contributor

This PR updates the favicons package.
There are a number of reasons for this update, but the main ones I found were:

  • newer versions no longer have phantomjs as a transitive dependency. This removes a big burden from npm installs and increases compatibility (phantomjs builds aren't available for all architectures)
  • newer version supports favicons for more platforms

The second bullet point was actually what made me update the "mocks" for the data we expect. Basically, by default, the package produces more favicons and more html.

It would probably be safer to major bump this package after merging this, since favicons was also major bumped.

@miguelcobain
Copy link
Contributor Author

miguelcobain commented Sep 21, 2021

@davewasmer @Exelord ~1 year later, this PR is still pretty much up to date and it would mean a big win for everybody. It's also a very easy win. :)

What can I do to help getting this merged?
Thank you.

@Exelord
Copy link
Collaborator

Exelord commented Sep 21, 2021

Hi Miguel,

Somehow I missed that notification, sorry :/

We need to ensure the 6.0 does not bring any breaking change in the config as well. The CI is not setup so that might be also a next step before.
Also we could update it to 6.2.2 to make safe guard against this year issues 😅

@Exelord
Copy link
Collaborator

Exelord commented Sep 21, 2021

Once tests will be green, I'm happy to merge :)

@miguelcobain
Copy link
Contributor Author

@Exelord looks like I need your approval for the github workflow to run.

@miguelcobain
Copy link
Contributor Author

I went ahead and removed the travis.yaml file.

@miguelcobain
Copy link
Contributor Author

@Exelord increased timeouts. GH actions are probably slower? 🤷‍♂️
Tests are all green locally. Can you approve workflows again, please?

@miguelcobain
Copy link
Contributor Author

miguelcobain commented Sep 21, 2021

@Exelord Yay. Tests are 💚 .

@Exelord Exelord merged commit 6fd0159 into davewasmer:master Sep 21, 2021
@Exelord
Copy link
Collaborator

Exelord commented Sep 21, 2021

Thanks a lot! <3

@miguelcobain
Copy link
Contributor Author

@Exelord my pleasure.
When can we expect a release? 😃

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