Skip to content

Commit

Permalink
Refactor (and remove) check_... commands
Browse files Browse the repository at this point in the history
* Remove the old dashboard runner_factory
* These blocklists are literally all empty. Begone!
* Just use the profile directly
* Now all the check_... things are parallel.
* Replace check_commands with a list
(issue #3188 / PR #3218)
  • Loading branch information
simoncozens authored Mar 26, 2021
1 parent 59f71ec commit bd724c4
Show file tree
Hide file tree
Showing 19 changed files with 37 additions and 198 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ A more detailed list of changes is available in the corresponding milestones for
- Axis Registry has been updated to commit https://github.com/google/fonts/tree/6418bd97834330f245cce4131ec3b8b98cb333be which includes changes to the `opsz` axis.
- Format status output to improve readability. (issue #2052)
- Move reporter-specific write logic to reporters, simplify argparse (PR #3206)
- Profile-specific `fontbakery.commands.check_...` removed and replaced with a call to `check_profile` with the appropriate profile. (PR #3218)
- HTML reporter parses and renders markdown. (PR #3212)

### New Profile
- new Type Network profile for checking some of their new axis proposals (issue #3130)
- **[com.google.fonts/check/kern_table]:** add FAIL when non-character glyph present, WARN when no format-0 subtable present.
- Created a Type Network profile for checking some of their new axis proposals (issue #3130)

### Bugfixes
- **license** condition now assumes that all license files in a given project repo are identical if more than one is found. With that some checks wont be skipped. We should have a fontbakery check to ensuring that assumption is valid, though. (issue #3172)
Expand All @@ -24,6 +24,7 @@ A more detailed list of changes is available in the corresponding milestones for
- **[com.google.fonts/check/missing_small_caps_glyphs]:** Check small caps glyphs are available (issue #3154)

### Changes to existing checks
- **[com.google.fonts/check/kern_table]:** add FAIL when non-character glyph present, WARN when no format-0 subtable present.
- **[com.google.fonts/check/gf-axisregistry/fvar_axis_defaults]:** Only check axes which are in the GF Axis Registry (PR #3217)
- **[com.google.fonts/check/mandatory_avar_table]:** Update rationale to mention that this check may be ignored if axis progressions are linear.
- **[com.google.fonts/check/integer_ppem_if_hinted]:** Format message with newlines.
Expand Down
36 changes: 31 additions & 5 deletions Lib/fontbakery/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,50 @@
import pkgutil
import runpy
import sys
from importlib import import_module

import fontbakery.commands
from fontbakery.commands.check_profile import main as check_profile_main

CLI_PROFILES = [
"adobefonts",
"fontval",
"googlefonts",
"notofonts",
"opentype",
"typenetwork",
"ufo_sources",
"universal",
]


def run_profile_check(profilename):
module = import_module("fontbakery.profiles.%s" % profilename)
sys.exit(check_profile_main(module.profile))


def main():
subcommands = [
pkg[1].replace("_", "-")
for pkg in pkgutil.walk_packages(fontbakery.commands.__path__)
]
pkg[1] for pkg in pkgutil.walk_packages(fontbakery.commands.__path__)
] + ["check_" + prof for prof in CLI_PROFILES]

subcommands = [command.replace("_", "-") for command in sorted(subcommands)]

if len(sys.argv) >= 2 and sys.argv[1] in subcommands:
# Relay to subcommand.
subcommand = sys.argv[1]
subcommand_module = subcommand.replace("-", "_")
sys.argv[0] += " " + subcommand
del sys.argv[1] # Make this indirection less visible for subcommands.
runpy.run_module(
"fontbakery.commands." + subcommand_module, run_name='__main__')
if (
subcommand_module.startswith("check_")
and subcommand_module[6:] in CLI_PROFILES
):
run_profile_check(subcommand_module[6:])
else:
runpy.run_module(
"fontbakery.commands." + subcommand_module, run_name="__main__"
)
else:
description = (
"Run fontbakery subcommands. Subcommands have their own help "
Expand Down
24 changes: 0 additions & 24 deletions Lib/fontbakery/commands/check_adobefonts.py

This file was deleted.

22 changes: 0 additions & 22 deletions Lib/fontbakery/commands/check_fontval.py

This file was deleted.

22 changes: 0 additions & 22 deletions Lib/fontbakery/commands/check_googlefonts.py

This file was deleted.

22 changes: 0 additions & 22 deletions Lib/fontbakery/commands/check_notofonts.py

This file was deleted.

13 changes: 0 additions & 13 deletions Lib/fontbakery/commands/check_opentype.py

This file was deleted.

5 changes: 0 additions & 5 deletions Lib/fontbakery/commands/check_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,6 @@ def error(message): raise ArgumentParserError(message)
raise Exception(f"Can't get a profile from {imported}.")
return profile

# This stub or alias is kept for compatibility (e.g. check-commands, FontBakery
# Dashboard). The function of the same name previously only passed on all parameters to
# CheckRunner.
runner_factory = CheckRunner

def main(profile=None, values=None):
# profile can be injected by e.g. check-googlefonts injects it's own profile
add_profile_arg = False
Expand Down
23 changes: 0 additions & 23 deletions Lib/fontbakery/commands/check_typenetwork.py

This file was deleted.

23 changes: 0 additions & 23 deletions Lib/fontbakery/commands/check_ufo_sources.py

This file was deleted.

22 changes: 0 additions & 22 deletions Lib/fontbakery/commands/check_universal.py

This file was deleted.

2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/adobefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
profile_imports = ('fontbakery.profiles.universal',)
profile = profile_factory(default_section=Section("Adobe Fonts"))

BLOCKLISTS = {}

ADOBEFONTS_PROFILE_CHECKS = \
UNIVERSAL_PROFILE_CHECKS + [
'com.adobe.fonts/check/family/consistent_upm',
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/fontval.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
profile_imports = ['.shared_conditions']
profile = profile_factory(default_section=Section("Checks inherited from Microsoft Font Validator"))

BLOCKLISTS = {}

@check(
id = 'com.google.fonts/check/fontvalidator'
)
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/googlefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
profile_imports = ('fontbakery.profiles.universal',)
profile = profile_factory(default_section=Section("Google Fonts"))

BLOCKLISTS = {}

METADATA_CHECKS = [
'com.google.fonts/check/metadata/parses',
'com.google.fonts/check/metadata/unknown_designer',
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/notofonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
CMAP_TABLE_CHECKS + \
OS2_TABLE_CHECKS

BLOCKLISTS = {}

@check(
id = 'com.google.fonts/check/cmap/unexpected_subtables',
rationale = """
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/typenetwork.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
'io.github.abysstypeco/check/ytlc_sanity'
]

BLOCKLISTS = {}

@check(
id='io.github.abysstypeco/check/ytlc_sanity',
rationale="""
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/ufo_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
from fontbakery.section import Section
from fontbakery.profile import Profile

BLOCKLISTS = {}

class UFOProfile(Profile):

def setup_argparse(self, argument_parser):
Expand Down
2 changes: 0 additions & 2 deletions Lib/fontbakery/profiles/universal.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@
'com.google.fonts/check/rupee'
]

BLOCKLISTS = {}

@check(
id = 'com.google.fonts/check/name/trailing_spaces',
)
Expand Down
4 changes: 3 additions & 1 deletion tests/commands/test_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

def test_list_subcommands_has_all_scripts():
"""Tests if the output from running `fontbakery --list-subcommands` matches
the fontbakery scripts within the bin folder."""
the fontbakery scripts within the bin folder and the promoted profiles."""
import fontbakery.commands
from fontbakery.cli import CLI_PROFILES
commands_dir = os.path.dirname(fontbakery.commands.__file__)

scripts = [
f.rstrip(".py").replace("_", "-")
for f in os.listdir(commands_dir)
if (f.endswith(".py") and not f.startswith('_'))
]
scripts = scripts + [ ("check-"+i).replace("_", "-") for i in CLI_PROFILES ]
subcommands = subprocess.check_output(
['fontbakery', '--list-subcommands']).decode().split()
assert sorted(scripts) == sorted(subcommands)
Expand Down

0 comments on commit bd724c4

Please sign in to comment.