Fontbakery: Reevaluate check statuses for all googlefonts profile checks.

[m4rc1e]

I'd like us at some point to reevaluate all the statuses for the googlefonts profile. Imo a FAIL should mean this font is going to break the internet, not this copyright string is incorrect.

Once we've rejigged the statuses, I can finally get the google/fonts ci to use the FB results to pass or fail a pr.

[graphicore]

The old suggested feature "per project check result overrides" via a yaml/config file would help with that.

[felipesanches]

I think that FAILs have the very important purpose of enforcing adherence to a QA spec. While WARNs are meant to point out potential problems that need attention but may be ignored based on a case-by-case evaluation.

I do not want to change that semantics. If we make some changes, we should understand that we'll be effectively changing the QA spec that such changes encode.

I know that enforcing copyright string formats may sound like "bossing around" too much. But if that fulfills the purpose of reducing the noise and making the font data more homogeneous, maybe it is worth doing so. (It may even have important legal implications, I would guess, even though I am not aware of any of that) It is a matter of deciding what are the quality aspects we want to achieve for the fonts collection.

This obviously applies to all FB check profiles (either universal or vendor-specific ones), but I feel that @m4rc1e may be specially concerned with the googlefonts profile and how does it encode the Google Fonts QA specs.

Since there's a tendency of putting less effort on addressing WARNs, any problem labeled that way will likely remain not fixed until much later into the future. So here's one possible approach I'd suggest: we might choose to make the QA specs evolve gradually, by initially keeping some "prospective" problems at WARN level, but then upgrade each into a FAIL as we decide to address them in batch for the entire collection.

I would like to hear @davelab6 's thoughts on that as well.

[RosaWagner]

Isn't it possible to tag the checks specific to GF spec?

FAIL = font file is broken (universal)
WARN = font file is not broken but it may be a mistake so you may want to look into this (universal and google specific)
RQD = font file not broken but this is required by Google Font (specific to google only) (rdq=required)

[felipesanches]

I wouldn't add that extra level. FAIL already has that meaning. And we don't want to make FontBakery vendor-biased. Placing checks on the googlefonts profile is enough to indicate that it is a check for a GFonts-specific issue. And it works the same for any other vendor profile.

[RosaWagner]

I am speaking about google profile, but it could be a level specific to any vendor. Let's say I suggest "RQD = required by vendor profile". Right now, when I use fontbakery through gftools, it looks like all the checks are specific to Google (they all have this link: com.google.fonts/check/…). But I get what you mean.

[felipesanches]

The reverse domain in the check-id indicates who contributed the initial check implementation. Many universal profile checks have the googlefonts domain in their ID because they were initially implemented by people involved with the Google Fonts project. But we also have some Dalton Maag and Adobe ones in there.

If we had a REQUIRED level for check results, I would not know what's left to distinguish a FAIL from a WARN. I think we really do not need such additional level.

[graphicore]

Right now, when I use fontbakery through gftools, it looks like all the checks are specific to Google

We could change that. We know for each check the profile of it.

All in all I agree with Felipe so far.

[graphicore]

We could change that.

We could change that in reporting, I mean. It's just a UI decision in a way.

[RosaWagner]

Ok, so no RQD ;)

Since there's a tendency of putting less effort on addressing WARNs

I think this is because there is so much checks in FAILs that now the FAIL mean important and WARN means less important. If FAIL = broken and WARN = not broken but, then the report is mostly made of warnings, and you try to fix them all equally. Then if it is needed to give priority to google spec checks, a flag could be added to the UI as @graphicore suggested. So then we come back to the important/less important thing, but at least we would know when the font is actually broken and when it still functional?

Example
FAIL : PS name has more than 29 characters (= font can't be used on windows so it is broken)
WARN : PS name has trailing spaces (= font works but it is probably a mistake)
WARN : !!Google Flag!! Name is not canonical to spec (= font works but won't be accepted by vendor)

[vv-monsalve]

We could change that in reporting

Agree. I was writing this:

Many universal profile checks have the googlefonts domain in their ID because they were initially implemented by people involved with the Google Fonts project

Maybe not an additional level, but reading now your explanation, I understand why sometimes it is complex to distinguish them; theuniversal checks from Google (as a vendor) related ones.

Since there's a tendency of putting less effort on addressing WARNs, any problem labeled that way will likely remain not fixed until much later into the future.

Because of this, I would also agree on keeping the semantics on FAILS and WARNS

Could it be a specific label in the message of the check?

I think it would be helpful for the users to differentiate them, a U FAIL related to a universal check, something that would break the internet from a GF FAIL related to a Google fonts specific test, something that could break the API or the fonts in it. It would apply to any other vendor, I guess.

[RosaWagner]

I think it would be helpful for the users to differentiate them, a U FAIL related to a universal check, something that would break the internet from a GF FAIL related to a Google fonts specific test, something that could break the API or the fonts in it. It would apply to any other vendor, I guess.

That works too if you want FAIL=required (either from vendor as for universal, wether the font is broken are not) and WARN=recommended.

[davelab6]

I filed an issue long back to add priority properties to checks so they can be stack ranked, and making metaprofiles seems also I useful.

[felipesanches]

we do have that kind of metadata in the source code (such as this: https://github.com/googlefonts/fontbakery/blob/master/Lib/fontbakery/profiles/googlefonts.py#L1090) but it has never been maintained, so its current tagging is imprecise and it has never been exposed to user interfaces. It is merely hanging out there in the source code.

Since it is inacurate and abandoned, I'd advocate for either a serious revamping of it or simply ditching it and keeping the system simple.

[felipesanches]

I gotta say I'm much more inclined to the "keep it simple" principle right now ;-D

[felipesanches]

The PriorityLevel class is defined at https://github.com/googlefonts/fontbakery/blob/2166cc2738656ee87dc19cd62dc4c7464cb4aa92/Lib/fontbakery/constants.py#L310-L316

If we actually adopted it thoughout our codebase we'd still have to decide which priority level to use as criteria for passing or not passing a continuous integration build such as on Travis.

So, I'd advocate to completely remove that class and keep the criteria as simple as possible, that is... any FAIL triggers a broken build, no FAIL means a clean build (even if with a few - or many! - WARNs)

[felipesanches]

Keep it simple, yay!!!

“the urge to destroy is also a creative urge”
https://medium.com/@rachelthompson_15193/the-urge-to-destroy-is-also-a-creative-urge-911c5650e395