fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
558 stars 103 forks source link

port checks from the AFDKO comparefamily tool to FontBakery #1992

Open cjchapman opened 6 years ago

cjchapman commented 6 years ago

As Thomas Phinney mentioned in the TypeCon 2018 MFDOS meeting: it would be great to include checks from the AFDKO comparefamily tool in FontBakery.

Here's a link to the code in GitHub: https://github.com/adobe-type-tools/afdko/blob/develop/python/afdko/comparefamily.py

cjchapman commented 6 years ago

We have this related issue in the AFDKO repo: https://github.com/adobe-type-tools/afdko/issues/501

I'm happy to work with you all to port the checks from the AFDKO into a FontBakery specification.

felipesanches commented 6 years ago

A first step here would be to list all the available checks giving them a short name as well as a longer rationale description. With that we can decide which id to use for each check. But they should probably be called something like com.adobe/check/keyword or maybe com.adobe.typetools/check/keyword or even com.adobe.type/check/keyword

Then we can write the boilerplate for these new checks and start refactoring snippets of the code into the boilerplate.

And finally we'd ideally expect code-tests to ensure the checks work properly I can help with that as well.

felipesanches commented 6 years ago

@cjchapman Please take a look at the example of porting a check from AFDKO into FontBakery that I have just pushed on pull request #2064

Please let me know if you have doubts on the code-style and architecture.

felipesanches commented 6 years ago

Also we should write code-tests. I may take some time to also write an example of that here later next week.

felipesanches commented 6 years ago

@cjchapman Also, it is important to understand the rationale behind these check routines. For instance, the first one that I ported is lacking a description of why 63 is the longest allowed string length for name id 18 and also why the strings should be unique in their first 32 characteres.

Which specific software requires that? What kind of problem is know to be caused by fonts that do not respect those requirements? We should be collecting this kind of info and placing it in the "rationale" field of the @check decorator.

felipesanches commented 6 years ago

These are all of the "single face tests" in AFDKO's comparefont.py:

felipesanches commented 6 years ago

And these are all of the "family tests":

felipesanches commented 6 years ago

We should document the reasoning behind each of all of these checks ;-)

It seems that some of those are similar to checks we already have in the Google Fonts specification. So it would also be nice to compare and understand case-by-case why do we sometimes have slightly different criteria.

felipesanches commented 6 years ago

@miguelsousa and @cjchapman, here are some thoughts abour SingleTest 1 ("Length overrun check for name ID 18. Max 63 characters, must be unique within 31 chars."):

felipesanches commented 6 years ago

Please note that the example of porting the "SingleTest 1" check into Font Bakery is probably incorrectly implemented. Use that example only as a demonstration of the API and code style, but the actual implementation should be reviewed and fixed (for instance, taking care of issues like what I described in my previous comment above: https://github.com/googlefonts/fontbakery/issues/1992#issuecomment-427253635)

tphinney commented 4 years ago

Fwiw, I know the rationale for a lot of the ”unexplained” checks. I could spend some time going over these. Some are simply spec-based. Others are due to app or OS limitations.

tphinney commented 4 years ago

To discuss individual checks, I could:

Preferences?

felipesanches commented 4 years ago

I'd prefer a shared spreadsheet to begin with and then gradually open individual issues for each proposed new check only when we feel it is worthy working on them (i.e. after we figure out good check-id, rationale, etc) so that an individual issue would be actually ready for someone to fix.

tphinney commented 4 years ago

I’m starting a spreadsheet. Once I have added enough commentary to be interesting, I will share the link and we can discuss.