-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[google_fonts] Update to v8.0.0 with new and removed fonts #10785
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
Conversation
`dart generator/generator.dart` adds support for numerous new fonts and removed several deprecated or replaced fonts. This corresponds to directory 11, generated in September 2025.
two fonts, Roboto and Saira, offer multiple width variants, exclude those for now and default to the right variant
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.
Code Review
This pull request updates the Google Fonts package to version 8.0.0, introducing a variety of new fonts while removing some deprecated ones. The changes primarily involve auto-generated files resulting from the font update. Additionally, the generator script has been modified to enhance the diff output format and to implement a more sophisticated font deduplication logic that now considers font widths. My review includes a couple of suggestions to improve the maintainability and performance of the generator script.
| @@ -1,3 +1,30 @@ | |||
| ## 8.0.0 | |||
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.
It feels unfortunate to introduce another breaking release so soon after 7.0.0. Is there any opportunity to not remove the two fonts and keep them around, maybe in a deprecated state for a while?
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.
I would lean towards not since it's not worth the additional tech debt. At the same time, we've been adhering to the convention that any font removal results in a major version bump. Also, I've gotten confirmation from the fonts team that they'll generate this quarterly, so we could consider this one exceptionally 2 months early (should land the woff2 update first) how does that sound?
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.
It feels unfortunate to introduce another breaking release so soon after 7.0.0. Is there any opportunity to not remove the two fonts and keep them around, maybe in a deprecated state for a while?
As it currently stands on how releases are done for this package and as a user/dev, in my experience, being worried about a major (breaking) release, based on time scale shouldn't even be a factor. It's better to give users the updates ASAP; they can choose to update or not, and at least they'll know about the breaking changes sooner than later to make appropriate adjustments.
I would lean towards not since it's not worth the additional tech debt. At the same time, we've been adhering to the convention that any font removal results in a major version bump. Also, I've gotten confirmation from the fonts team that they'll generate this quarterly, so we could consider this one exceptionally 2 months early (should land the woff2 update first) how does that sound?
I'm also on board that font removals should be a breaking change too. The whole package is about fonts and when one is removed, it could break a project.
A way around it for both the points I mentioned is to change how releases are done. My suggestion would be to add a @deprecated annotation for fonts that are to be removed and eventually remove them on a scheduled time interval. Then you can choose when to do a major release.
| /// | ||
| /// See: | ||
| /// * https://fonts.google.com/specimen/Amarna | ||
| static TextStyle amarna({ |
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.
Kind of weird, Github seems to have dropped syntax highlighting.. selectively.
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.
TIL syntax highlighting is limited to ~512 KB
|
Thank you for sharing your perspective @l1qu1d! Much appreciated. I think it makes sense. |
Piinks
left a comment
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.
This LGTM once the merge conflicts are resolved (changelog, pubspec).
I understand this is generated by the tool, and would take investment in reworking the tool to deprecate over its current behavior of removing.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
flutter/packages@9010299...5af5f50 2026-01-23 6655696+guidezpl@users.noreply.github.com Update Google Fonts to v8.0.0 with new and removed fonts (flutter/packages#10785) 2026-01-22 stuartmorgan@google.com [pigeon] Modernize Obj-C generation headers (flutter/packages#10857) 2026-01-22 47866232+chunhtai@users.noreply.github.com Fixes sync_release_pr workflow yaml formatting (flutter/packages#10855) 2026-01-22 robert.odrowaz@leancode.pl [camera_avfoundation] Wrappers swift migration - part 6 (flutter/packages#10752) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
dart generator/generator.dartadds support for numerous new fonts and removed several deprecated or replaced fonts. This corresponds to directory 12, generated in January 2026 (generated quarterly from now on).Updated the diff generator to match the current
CHANGELOG.mdformat.More duplicates were identified when initially generating. After investigation, I found that 2 fonts were producing combinations with multiple widths. This PR modifies
_deduplicateFontsto remove these non-standard width (100) variants.Fixes flutter/flutter#180333