fontbakery icon indicating copy to clipboard operation
fontbakery copied to clipboard

`Many simple checks` versus `fewer larger checks`

Open felipesanches opened this issue 1 year ago • 5 comments

@simoncozens made a suggestion at https://github.com/fonttools/fontbakery/issues/4418#issuecomment-2024676780 and then also hinted at the idea when posting https://github.com/fonttools/fontbakery/issues/4729#issue-2303588008

When we think of some new thing which needs checking, our response is normally to add another check for it. This means we have a large number of small, discrete checks, which individually are easy to understand and reason about but which don't necessarily "communicate" with one another. I would prefer to move towards a smaller number of checks which tell you everything about a particular topic - i.e. license URLS in all the different places they can appear - because then they can ensure consistency together.

felipesanches avatar May 22 '24 06:05 felipesanches

I agree, in principle, but I fear clarity of rationale for each check validation may be lost.

There are some checks that really validate the same kind of thing in multiple places, with the exact same rationale. For those it would be fine to clump everything in a single check.

But there are also small checks that have different reasoning each, such as the many aspects that we validate on different fields of the METADATA.pb file. Clumping those together would make it more complicated to convey to users what's the reasoning for each specific thing being checked. And it would also result in multiple "proposed" links to issue tracker entries.

felipesanches avatar May 22 '24 06:05 felipesanches

I suggest we start with one initial proposal of clumping together a given set of checks, and we see how we feel about it. If we like the result, we can move on to refactor additional sets of checks.

@simoncozens, which ones would you like to propose clumping together at first?

felipesanches avatar May 22 '24 06:05 felipesanches

Description/article. It's a GF-only thing so it only affects our process, and it's not as sprawling as metadata.pb.

simoncozens avatar May 22 '24 06:05 simoncozens

I'd personally like to do away with the little checks tbh. However, this isn't a hill I'm willing to die on.

I'd much prefer it is there was a single check for METADATA.pb that simply compares the current metadata.pb against a generated one, since we shouldn't be editing these by hand anyway (apart from the designer name, stroke etc).

m4rc1e avatar May 22 '24 09:05 m4rc1e

Just to keep track of a few checks I have been merging as I look at porting them:

  • DSIG is now just an unwanted table, com.google.fonts/check/dsig moves into com.google.fonts/check/unwanted_tables
  • com.google.fonts/check/unique_glyphnames can be checked at the same time as com.google.fonts/check/valid_glyphnames
  • com.google.fonts/check/metadata/parses, com.google.fonts/check/metadata/designer_values, com.google.fonts/check/metadata/date_added, com.google.fonts/check/metadata/category, com.google.fonts/check/metadata/menu_and_latin, com.google.fonts/check/metadata/subsets_order, com.google.fonts/check/metadata/single_cjk_subset are all standalone METADATA.pb validation checks, can all be done at the same time.
  • com.google.fonts/check/varfont/regular_wght_coord, com.google.fonts/check/varfont/regular_wdth_coord, com.google.fonts/check/varfont/regular_slnt_coord, com.google.fonts/check/varfont/regular_ital_coord, com.google.fonts/check/varfont/regular_opsz_coord can all be checked at the same time with a dictionary of regular coordinate values; similarly for the range checks in opentype/fvar.py.

simoncozens avatar Aug 22 '24 07:08 simoncozens