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

More rationales, better reporting #4570

Merged
merged 36 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
79b0171
Deprecated family/panose_proportion check
simoncozens Mar 1, 2024
1112cc9
[utils] added show_inconsistencies
felipesanches Mar 5, 2024
36588a2
Add rationale to panose_familytype and improve message when there are…
felipesanches Mar 5, 2024
ac1e0ad
"Critical" failures are now FATAL-level...
simoncozens Mar 1, 2024
df9c95f
Add rationale for xAvgCharWidth check
simoncozens Mar 1, 2024
fd56122
Improve fsselection_matches_macstyle reporting
simoncozens Mar 1, 2024
5589e48
Add rationale for OS/2.fsSelection check
simoncozens Mar 1, 2024
65fea4d
This is a FATAL now
simoncozens Mar 1, 2024
cfa7e2b
Add (dummy) rationale to questionable check
simoncozens Mar 2, 2024
cdc0f8a
Add rationale to ttx_roundtrip check
simoncozens Mar 2, 2024
04c63c5
Add rationale for a couple font version checks
simoncozens Mar 2, 2024
f57188c
Add rationale for a couple glyf table checks
simoncozens Mar 2, 2024
d40e596
Add rationale to a gpos table check
simoncozens Mar 2, 2024
16dd330
Add various other check rationales
simoncozens Mar 2, 2024
71db96c
Add check rationales for all universal checks
simoncozens Mar 2, 2024
cbd7122
Add some rationales for metadata checks
simoncozens Mar 2, 2024
04baba1
Improved display of detected inconsistencies on metadata/copyright
simoncozens Mar 2, 2024
cd6cad5
Improved display of detected inconsistencies on metadata/familyname
simoncozens Mar 3, 2024
b1b1f8a
Add rationale for check/fontval
simoncozens Mar 3, 2024
f159445
Add rationale for a couple name checks on gfonts profile
simoncozens Mar 3, 2024
7485976
Add rationale for equal_codepoint_coverage check
simoncozens Mar 3, 2024
abe2b02
Add rationale for description/min_length
simoncozens Mar 3, 2024
bd4a136
Add a bunch more metadata rationales
simoncozens Mar 4, 2024
0a1f47a
Add a couple gfonts copyright rationales
simoncozens Mar 4, 2024
055fc60
Add rationale for has_ttfautohint_params
simoncozens Mar 4, 2024
67cef1f
Improve rationale for hinting/smart_dropout
simoncozens Mar 4, 2024
07f8df4
Add rationale for production_glyphs_similarity
simoncozens Mar 4, 2024
c5afddd
Add rationale for version_bump
simoncozens Mar 4, 2024
7df2b6b
Add rationale for family/has_license
simoncozens Mar 4, 2024
dc4622a
Add rationale for metadata/license
simoncozens Mar 4, 2024
7348060
Add rationale for check/name/unwanted_chars
simoncozens Mar 4, 2024
6f94bc3
Add rationale for repo/dirname_matches_nameid_1 check
simoncozens Mar 4, 2024
881a2fc
Add more gfonts metadata rationales
simoncozens Mar 4, 2024
45499e0
Add rationale for ufolint check
simoncozens Mar 4, 2024
846974b
Add a code-test to ensure all checks have rationales
simoncozens Mar 4, 2024
0b9963b
alphabetical re-ordering of changelog entries
felipesanches Mar 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,91 @@ A more detailed list of changes is available in the corresponding milestones for
- v0.12.0a1 (2024-Feb-14)
- v0.12.0a2 (2024-Feb-21)

### New utilily functions
- **fontbakery.utils.show_inconsistencies**: Display an 'inconsistencies dictionary' as a bullet list. (PR #4570)

### Deprecated checks
#### Removed from the Open Type profile
- **[com.google.fonts/check/family/panose_proportion]:** (issue #4083)

### Changes to existing checks
#### On the Google Fonts profile
- **[com.google.fonts/check/description/min_length]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/family/equal_codepoint_coverage]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/family/has_license]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/has_ttfautohint_params]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/broken_links]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/canonical_weight_value]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/canonical_style_names]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/copyright]:** Added rationale and improve display of detected inconsistencies. (PR #4570)
- **[com.google.fonts/check/metadata/copyright_max_length]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/familyname]:** Added rationale and improve display of detected inconsistencies. (PR #4570)
- **[com.google.fonts/check/metadata/has_tags]:** Also fetch family tagging data from a second Google Sheet (Submissions from designers via form: https://forms.gle/jcp3nDv63LaV1rxH6) (PR #4576)
- **[com.google.fonts/check/metadata/has_regular]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/italic_style]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/license]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/match_filename_postscript]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/match_fullname_postscript]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/match_name_familyname]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/match_weight_postscript]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/menu_and_latin]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/nameid/copyright]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/nameid/family_and_full_names]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/nameid/family_name]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/nameid/full_name]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/nameid/post_script_name]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/normal_style]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/valid_filename_values]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/valid_full_name_values]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/valid_post_script_name_values]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/regular_is_400]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/reserved_font_name]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/subsets_order]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/unique_full_name_values]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/unique_weight_style_pairs]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/unknown_designer]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/metadata/valid_name_values]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/name/mandatory_entries]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/name/unwanted_chars]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/name/version_format]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/production_glyphs_similarity]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/repo/dirname_matches_nameid_1]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/smart_dropout]:** Improve rationale with better fixing instructions. (PR #4570)
- **[com.google.fonts/check/version_bump]:** Added rationale. (PR #4570)

#### On the UFO Sources profile
- **[com.daltonmaag/check/ufolint]:** Added rationale. (PR #4570)

#### On the FontValidator profile
- **[com.google.fonts/check/fontvalidator]:** Added rationale. (PR #4570)

#### On the Universal profile
- **DISABLED - [com.google.fonts/check/legacy_accents]:** This is one of the checks that we probably should run on the sources instead of binaries. (https://github.com/fonttools/fontbakery/issues/3959#issuecomment-1822913547)
- **[com.google.fonts/check/name/trailing_spaces]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/ots]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/ttx_roundtrip]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/whitespace_glyphs]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/whitespace_ink]:** Added rationale. (PR #4570)

#### On the Open Type Profile
- **[com.google.fonts/check/family/equal_font_versions]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/family/equal_unicode_encodings]:** Added rationale, even though this is a questionable check, which we may simply deprecate, as we're not sure why would we expect what it checks. (PR #4570)
- **[com.google.fonts/check/family_naming_recommendations]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/family/panose_familytype]:** Add rationale and improve message when there are inconsistencies. (PR #4570)
- **[com.google.fonts/check/font_version]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/fsselection]:** Added rationale. (PR #4570)
- **[com.adobe.fonts/check/fsselection_matches_macstyle]:** Improve log messages to include the bit values. (PR #4570)
- **[com.google.fonts/check/glyf_unused_data]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/gpos_kerning_info]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/layout_valid_feature_tags]:** Updated the check to allow valid private-use feature tags. (issue #4544)
- **[com.google.fonts/check/varfont/family_axis_ranges]:** Updated the check to skip fonts without fvar tables. (issue #4554)
- **[com.google.fonts/check/loca/maxp_num_glyphs]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/mac_style]:** Skip if font style can not be determined. (issue #4349)
- **[com.google.fonts/check/maxadvancewidth]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/name/no_copyright_on_description]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/points_out_of_bounds]:** Added rationale. (PR #4570)
- **[com.adobe.fonts/check/postscript_name]:** Added rationale. (PR #4570)
- **[com.google.fonts/check/varfont/family_axis_ranges]:** Updated the check to skip fonts without fvar tables. (issue #4554)
- **[com.google.fonts/check/xavgcharwidth]:** Added rationale. Also, when glyphs needed to compute xAvgCharWidth are missing, it is considered critical, so it is now results in a FATAL-level result. (PR #4570)


## 0.12.0a2 (2024-Feb-21)
Expand Down
12 changes: 11 additions & 1 deletion Lib/fontbakery/checks/fontval.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,17 @@
from fontbakery.utils import exit_with_install_instructions


@check(id="com.google.fonts/check/fontvalidator", proposal="legacy:check/037")
@check(
id="com.google.fonts/check/fontvalidator",
proposal="legacy:check/037",
rationale="""
Microsoft Font Validator is a tool that can be used to check for
various problems with a font file. Fonts which report errors in
Microsoft Font Validator are likely to have problems in Microsoft
Windows applications. This check runs Microsoft Font Validator
on the font and reports any errors or warnings that it finds.
""",
)
def com_google_fonts_check_fontvalidator(font, config):
"""Checking with Microsoft Font Validator."""

Expand Down
10 changes: 10 additions & 0 deletions Lib/fontbakery/checks/googlefonts/copyright.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ def com_google_fonts_check_glyphs_file_font_copyright(glyphsFile):
id="com.google.fonts/check/metadata/copyright_max_length",
conditions=["font_metadata"],
proposal="legacy:check/104",
rationale="""
We check that the copyright notice within the METADATA.pb file is
not too long; if it is more than 500 characters, this may be an
indiciation that either a full license or the font's description
has been included in this field by mistake.
""",
)
def com_google_fonts_check_metadata_copyright_max_length(font_metadata):
"""METADATA.pb: Copyright notice shouldn't exceed 500 chars."""
Expand All @@ -113,6 +119,10 @@ def com_google_fonts_check_metadata_copyright_max_length(font_metadata):
id="com.google.fonts/check/metadata/nameid/copyright",
conditions=["font_metadata"],
proposal="legacy:check/155",
rationale="""
This check verifies that the copyright field in METADATA.pb matches the
contents of the name table nameID 0 (Copyright).
""",
)
def com_google_fonts_check_metadata_nameid_copyright(ttFont, font_metadata):
"""Copyright field for this font on METADATA.pb matches
Expand Down
8 changes: 8 additions & 0 deletions Lib/fontbakery/checks/googlefonts/description.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,14 @@ def com_google_fonts_check_description_valid_html(descfile, description):
id="com.google.fonts/check/description/min_length",
conditions=["description"],
proposal="legacy:check/005",
rationale="""
The DESCRIPTION.en_us.html file is intended to provide a brief overview of
the font family. It should be long enough to be useful to users, but not so
long that it becomes overwhelming.

We chose 200 bytes as a minimum length because it suggests that someone has
taken the time to write "something sensible" about the font.
""",
)
def com_google_fonts_check_description_min_length(description):
"""DESCRIPTION.en_us.html must have more than 200 bytes."""
Expand Down
7 changes: 7 additions & 0 deletions Lib/fontbakery/checks/googlefonts/family.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
id="com.google.fonts/check/family/equal_codepoint_coverage",
conditions=["are_ttf", "stylenames_are_canonical"],
proposal="https://github.com/fonttools/fontbakery/issues/4180",
rationale="""
For a given family, all fonts must have the same codepoint coverage.
This is because we want to avoid the situation where, for example,
a character is present in a regular font but missing in the italic style;
turning on italic would cause the character to be rendered either as a
fake italic (auto-slanted) or to show tofu.
""",
)
def com_google_fonts_check_family_equal_codepoint_coverage(fonts, config):
"""Fonts have equal codepoint coverage"""
Expand Down
9 changes: 9 additions & 0 deletions Lib/fontbakery/checks/googlefonts/hinting.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ def com_google_fonts_check_hinting_impact(font):
@check(
id="com.google.fonts/check/has_ttfautohint_params",
proposal="https://github.com/fonttools/fontbakery/issues/1773",
rationale="""
It is critically important that all static TTFs in the API which
were autohinted with ttfautohint store their TTFAutohint args in
the 'name' table, so that an automated solution can be made to
replicate the hinting on subsets, etc.
""",
)
def com_google_fonts_check_has_ttfautohint_params(ttFont):
"""Font has ttfautohint params?"""
Expand Down Expand Up @@ -321,6 +327,9 @@ def ttfautohint_version(values):

For more detailed info (such as other rules not enabled in this snippet),
please refer to the TrueType Instruction Set documentation.

Generally this occurs with unhinted fonts; if you are not using autohinting,
use gftools-fix-nonhinting (or just gftools-fix-font) to fix this issue.
""",
proposal="legacy:check/072",
)
Expand Down
11 changes: 11 additions & 0 deletions Lib/fontbakery/checks/googlefonts/hosted.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
id="com.google.fonts/check/version_bump",
conditions=["api_gfonts_ttFont", "github_gfonts_ttFont"],
proposal="legacy:check/117",
rationale="""
We check that the version number has been bumped since the last release on
Google Fonts. This helps to ensure that the version being PRed is newer than
the one currently hosted on fonts.google.com.
""",
)
def com_google_fonts_check_version_bump(
ttFont, api_gfonts_ttFont, github_gfonts_ttFont
Expand Down Expand Up @@ -55,6 +60,12 @@ def com_google_fonts_check_version_bump(
id="com.google.fonts/check/production_glyphs_similarity",
conditions=["api_gfonts_ttFont"],
proposal="legacy:check/118",
rationale="""
We check that the glyphs in the font are similar to the glyphs in the
version hosted on fonts.google.com. We do not expect updated fonts to
have exactly the same glyphs as the previous version, but we do expect
the changes to be minimal.
""",
)
def com_google_fonts_check_production_glyphs_similarity(
ttFont, api_gfonts_ttFont, config
Expand Down
10 changes: 10 additions & 0 deletions Lib/fontbakery/checks/googlefonts/license.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ def is_ofl(font):
id="com.google.fonts/check/family/has_license",
conditions=["gfonts_repo_structure"],
proposal="legacy:check/028",
rationale="""
A license file is required for all fonts in the Google Fonts collection.
This checks that the font's directory contains a file named OFL.txt or
LICENSE.txt.
""",
)
def com_google_fonts_check_family_has_license(licenses, config):
"""Check font has a license."""
Expand Down Expand Up @@ -443,6 +448,11 @@ def com_google_fonts_check_name_license_url(ttFont, familyname):
id="com.google.fonts/check/metadata/license",
conditions=["family_metadata"],
proposal="legacy:check/085",
rationale="""
The license field in METADATA.pb must contain one of the
three values "APACHE2", "UFL" or "OFL". (New fonts should
generally be OFL unless there are special circumstances.)
""",
)
def com_google_fonts_check_metadata_license(family_metadata):
"""METADATA.pb license is "APACHE2", "UFL" or "OFL"?"""
Expand Down
Loading
Loading