-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat: Update for regular icons set to 4.0.0 version #36
Conversation
Hey @robothemes! Thank you for the PR. This project was not updated for a while, so updating it to the latest material icons versions is quite nice. Before accepting this PR, I need to work on bringing back the CI so everything is properly tested. I created #37 and a v4.0.0 milestone to track this effort. I plan to cover several issues that have been open for some time as part of this release. I will try to add the CI in a few days, so we can properly test and merge these changes. Will ping you when it's ready, but you will need to rebase your fork and apply same changes again so the CI is taken into account. Thank you! |
Hello @robothemes! Sorry for the delay here. I just finished some pending tasks and this PR is unblocked now. You would need to rebase your changes as some changes are no longer required:
Then, the PR is good to go! |
Btw, this PR closes #39 |
Hey, @Angelmmiguel I'm in the process of moving my app off the material_design_icons gem which is now deprecated. I'd like to use this gem but about half the icons I tried to use were not supported. Any idea when this change will be ready? |
Hello @hawkes69, I'm glad you find I cannot ensure you a deadline, but I would like to have the next version published by the end of next week :). |
Thanks for the quick response @Angelmmiguel. I'm not in a huge rush I just wanted a ballpark so this is perfect. Thanks for the help! |
@Angelmmiguel do you have any update on when this will be done? |
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 the final PR @robothemes!
The PR LGTM. I was planning to drop EOT as I wanted to drop support for IE. So, it's fine to remove it in this PR too. I will create a separate task to remove it and associate it here.
However, there's a test that actually checks the presence of the EOT file, so you would need to remove it too: https://github.com/Angelmmiguel/material_icons/blob/master/spec/requests/material_icons_fonts_spec.rb#L7.
We're closer to close this @hawkes69 :)
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.
LGTM!
Hi. I use Material Icons set in our current project and it definitely misses new icons so I decided to update it to the latest version. It will be great if you can update your project.