[Reporters] Markdown reporter #1754
Comments
|
This now seems quite important to do soon, as I'd like to gear up to start using FB & dispatcher to hit a weekly target of PRs :) Turns out Github has a "expanding" markup that is really sweet!! |
|
oh! That's really nice! |
FAIL: DESCRIPTION.en_us.html must have less than 1000 bytes.com.google.fonts/check/006
FAIL: Fonts have consistent underline thickness?com.google.fonts/check/008
FAIL: Fonts have equal numbers of glyphs?com.google.fonts/check/011
FAIL: Fonts have equal glyph names?com.google.fonts/check/012
WARN: Checking OS/2 achVendID.com.google.fonts/check/018 with ((u'font[0]', '/var/folders/f7/2dqpt71s6f7b91z7_vbgykkm0000gn/T/tmpRiKnhs/Aleo-Bold.ttf'),)
Total
|
|
The comment above is a sketch of one possible markdown markup scheme we might use. |
|
What do you think, @m4rc1e ? |
|
Maybe put the total in a 2 row table? |
FAIL: DESCRIPTION.en_us.html must have less than 1000 bytes.com.google.fonts/check/006
FAIL: Fonts have consistent underline thickness?com.google.fonts/check/008
FAIL: Fonts have equal numbers of glyphs?com.google.fonts/check/011
FAIL: Fonts have equal glyph names?com.google.fonts/check/012
WARN: Checking OS/2 achVendID.com.google.fonts/check/018 with ((u'font[0]', '/var/folders/f7/2dqpt71s6f7b91z7_vbgykkm0000gn/T/tmpRiKnhs/Aleo-Bold.ttf'),)
Summary
|
|
Oh! Two rows! Not Columns... Makes sense :-) |
FAIL: DESCRIPTION.en_us.html must have less than 1000 bytes.com.google.fonts/check/006
FAIL: Fonts have consistent underline thickness?com.google.fonts/check/008
FAIL: Fonts have equal numbers of glyphs?com.google.fonts/check/011
FAIL: Fonts have equal glyph names?com.google.fonts/check/012
WARN: Checking OS/2 achVendID.com.google.fonts/check/018 with ((u'font[0]', '/var/folders/f7/2dqpt71s6f7b91z7_vbgykkm0000gn/T/tmpRiKnhs/Aleo-Bold.ttf'),)
Summary
|
|
I love this. |
|
The markdown output below is generated by my current implementation. It sorts check results, grouping them by result type. This will make it easier to read since FAILs will always be grouped on top. The order the other groups show up is less relevant. I originally wanted to have WARNs coming right after the FAILs, but the alphabetical order placed WARNs in the end and I actually endedup liking that, since typically SKIPs and INFOs will be very few and it they will likely be ignored, but it is nice to have them hanging there in the middle so they may catch the eyes at least briefly. ERRORs are huge problems with the code, so they should show up right on top of all results (but they are unlikely to happen as we improve the codebase). PASSES are omitted here by default, but they can be included by passing --verbose on the command-line. The output below was generated with the following command line: FAIL: DESCRIPTION.en_us.html must have less than 1000 bytes.com.google.fonts/check/006
FAIL: Copyright notice on METADATA.pb matches canonical pattern?com.google.fonts/check/102
FAIL: TTFAutohint x-height increase value is same as in previous release on Google Fonts?com.google.fonts/check/119
FAIL: METADATA.pb family.full_name and family.post_script_name fields have equivalent values ?com.google.fonts/check/096
FAIL: Check font has same encoded glyphs as version hosted on fonts.google.comcom.google.fonts/check/154
FAIL: Version number has increased since previous release on Google Fonts?com.google.fonts/check/117
INFO: EPAR table present in font?com.google.fonts/check/061
INFO: Familyname must be unique according to namecheck.fontdata.comcom.google.fonts/check/165
INFO: Check for font-v versioningcom.google.fonts/check/166
INFO: Show hinting filesize impact.com.google.fonts/check/054
| | data/test/merriweather/Merriweather-Regular.ttf | SKIP: METADATA.pb font.style "italic" matches font internals?com.google.fonts/check/106
SKIP: Check a static ttf can be generated from a variable font.com.google.fonts/check/174
WARN: Copyright notice on METADATA.pb does not contain Reserved Font Name?com.google.fonts/check/103
WARN: Check name table: FULL_FONT_NAME entries.com.google.fonts/check/159
WARN: Check if each glyph has the recommended amount of contours.com.google.fonts/check/153
The following glyphs do not have the recommended number of contours: Glyph name: uni20A9 Counters detected: 6 Expected: 1, 3, 4 or 7 WARN: Glyphs are similiar to Google Fonts version?com.google.fonts/check/118
WARN: Is font em size (ideally) equal to 1000?com.google.fonts/check/116
Summary
|
|
There currently are two shortcomings with the current implementation and I plan to fix these before merging the feature;
|
|
This also makes me wonder if I should be grouping check results by font file, adding sections with the corresponding filenames in the section headers on the markdown output |
felipesanches
added a commit
to felipesanches/fontbakery
that referenced
this issue
May 9, 2018
meant to be used on github pull-requests (likely automated by Marc Foley's tooling for onboarding fonts into the google/fonts git repo) (issue fonttools#1754)
felipesanches
modified the milestones:
0.4.1 - Future major release,
0.4.0 - First Python 3 release
If you still have the Status objects around, sorting is straight forward: |
|
Not on the Markdown reporter, since that is mainly targeting posting on Github. However, on the FB Dashboard, the INFO check results can be extremely interesting and should be available to the HTML report template.
Sounds good to me - something that should be clearly document in the README :) |
Well, actually the Markdown reporter must support all check results, and then it would be invoked with the
INFO is PASS status, but you included them in your sample, when they should not be :) This suggests something is wrong with However, knowing the total counts in the summary table for all checks (including INFO and PASS) is still useful. The table should really have 3 rows - labels, absolute numbers, and percentages :)
It is debatable if, when a type designer is responding to a check report, they will go check by check, or go to fix everything with each instance by instance. I believe Lasse's current dashboard report page assumes the latter, and I think that is reasonable. Therefore, I would propose
Also, your sample above uses Family ChecksFAIL: DESCRIPTION.en_us.html must have less than 1000 bytes (source)ERROR: Second check is this? (source)WARN: Second check is that other thing? (source) |
Maybe better than
|
|
hahaha! I hope that's a joke... (the hearts) |
|
No joke - use of color is a foundation of graphic design, and keeping all B&W is not sharp :) |
|
Sure. The hearts look a bit silly. But I agree that color is good. I even considered doing so previously but ended up not drafting that aspect. I think it may be possible to display the actual status names with these colors. And that, I think would be better than the emojis. |
|
Hmmm... So yeah... some websites suggest these things indeed:
|
|
maybe the circles and diamonds as less silly at least... |
|
and then I start to agree with you that perhaps using hearts for everything may be good for keeping it homogeneous. But I still wouldn't drop the actual status names, even with the aid of color. |
But we are silly! :)
Fair enough :) |
|
Can we bikeshed the status emoji 😀 ?
|
|
Thanks, Lasse! I loved it all! :-D |
Closed
|
What with the bakery theme, perhaps
|
I think 💤 because none of the others are face based, and the blue color is helpful as its an "info" level status I would also swap ERROR and FAIL, like this, because errors are when things are broken with the system, which is tragic and not the fault of the type designer, but when things FAIL that is a fire they need to put out :)
|
|
I like that! More options:
|
|
As no-one will likely be looking at PASS statuses, that's truly bike-shedding :-) |
|
but I'll adopt some of these, sure. |
|
🍩 as well |
|
But people will look at the PASS col label that will presumably follow
these conventions :)
|
[1] Family checks🔥 FAIL: DESCRIPTION.en_us.html must have less than 1000 bytes.
[36] Merriweather-LightItalic.ttf💔 ERROR: Checking with ftxvalidator.
💔 ERROR: Checking with Microsoft Font Validator.
🔥 FAIL: METADATA.pb font.name and font.full_name fields match the values declared on the name table?
🔥 FAIL: Copyright notice on METADATA.pb matches canonical pattern?
🔥 FAIL: TTFAutohint x-height increase value is same as in previous release on Google Fonts?
🔥 FAIL: Checks METADATA.pb font.name field matches family name declared on the name table.
🔥 FAIL: Fonts have equal numbers of glyphs?
🔥 FAIL: Fonts have equal glyph names?
🔥 FAIL: Check font has same encoded glyphs as version hosted on fonts.google.com
🔥 FAIL: Version number has increased since previous release on Google Fonts?
🔥 FAIL: Checking OS/2 usWinAscent & usWinDescent.
🔥 FAIL: Description strings in the name table must not exceed 100 characters.
ℹ️ INFO: EPAR table present in font?
ℹ️ INFO: Familyname must be unique according to namecheck.fontdata.com
ℹ️ INFO: Check for font-v versioning
ℹ️ INFO: Show hinting filesize impact.
| | data/test/merriweather/Merriweather-LightItalic.ttf | ℹ️ INFO: Font contains all required tables?
💤 SKIP: METADATA.pb font.style "normal" matches font internals?
💤 SKIP: Check a static ttf can be generated from a variable font.
💤 SKIP: FontForge checks.
💤 SKIP: FontForge validation outputs error messages?
💤 SKIP: Monospace font has hhea.advanceWidthMax equal to each glyph's advanceWidth?
💤 SKIP: The variable font 'ital' (Italic) axis coordinate must be zero on the 'Regular' instance.
💤 SKIP: The variable font 'wdth' (Width) axis coordinate must be 100 on the 'Regular' instance.
💤 SKIP: The variable font 'wght' (Weight) axis coordinate must be 700 on the 'Bold' instance.
💤 SKIP: The variable font 'slnt' (Slant) axis coordinate must be zero on the 'Regular' instance.
💤 SKIP: The variable font 'wght' (Weight) axis coordinate must be 400 on the 'Regular' instance.
💤 SKIP: The variable font 'opsz' (Optical Size) axis coordinate should be between 9 and 13 on the 'Regular' instance.
|
| 💔 ERROR | 🔥 FAIL | 💤 SKIP | ℹ️ INFO | 🥐 PASS | |
|---|---|---|---|---|---|
| 16 | 74 | 63 | 92 | 40 | 610 |
felipesanches
added a commit
to felipesanches/fontbakery
that referenced
this issue
May 12, 2018
meant to be used on github pull-requests (likely automated by Marc Foley's tooling for onboarding fonts into the google/fonts git repo) (issue fonttools#1754)
|
The above was the output with |
|
I don't get why you prefer sorting WARN to the end. |
Right now it is sorted by alphabetical order, which is not necessarily good, but also not terrible. I mentioned an insight regarding this at #1754 (comment):
I can improve this, though. |
|
The table should have 3 rows as above, a new final row with percentages, and as Lasse says the order of the results should be grouped the same as the order of cols in the table |
|
sure! I'm taking care of doing so |
felipesanches
added a commit
to felipesanches/fontbakery
that referenced
this issue
May 14, 2018
felipesanches
added a commit
to felipesanches/fontbakery
that referenced
this issue
May 14, 2018
felipesanches
added a commit
that referenced
this issue
May 14, 2018
meant to be used on github pull-requests (likely automated by Marc Foley's tooling for onboarding fonts into the google/fonts git repo) (issue #1754)
felipesanches
added a commit
that referenced
this issue
May 14, 2018
felipesanches
added a commit
that referenced
this issue
May 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It would be great to have a reporter which outputs markdown.
I often file FB reports to people's repos and google/fonts. Atm, I use a code block google/fonts#1502
The text was updated successfully, but these errors were encountered: