fonttools
Add human metadata to each check
Check result reports for individual font projects should not have "OK / ERROR / FAIL" messages and developer-level technical details.
We need descriptions of errors that are meaningful to font designers so that instead of a list of (perhaps cryptic) error messages, FB serves as a source of knowledge about how to do things in a proper way.
Therefore each check need detailed documentation metadata, written in simple english:
- [x] what common pitfalls does check address?
- [x] what went wrong with the font being tested? (the actual error message)
- [ ] what to do to fix it in source files (and ideally no "hotfix" methods.) The process to submit a PR to add to this should be easy. Eg, markdown files in the repo.
- [ ] links to exemplary valid fonts/projects
This should be done
- [ ] first for the tool that checks individual files,
- [ ] then for the tool that checks sets of files,
- [ ] then for the tool that checks families
As part of all of the above,
- [ ] add doc strings for each method in each scripts
@m4rc1e wrote,
It's fine not to have a doc string for check-ttf.py. Felipe's suggestion is good. However, this is a lot of work. Better logging is more appropriate for this script imo.
I would really like them on the fix scripts, e.g fontbakery-fix-gasp.py could have.
''' Set font gasp to 15, across all ranges. More info on can be found on the GF spec, https://github.com/googlefonts/gf-docs/blob/master/ProjectChecklist.md#gasp '''This pretty much summarises what the script is doing, without me having to read the code.
This is one exemple of cryptic error messages:
Regexp didn't match: 'Copyright\\s+\\(c\\)\\s+20\\d{2}.*\\(.*@.*.*\\)'
not found in u'Copyright (c) 2015, Marcelo Magalhaes'
It should read something like:
Copyright strings should look like:
Copyright (c) 2015 Author Name
Copyright notices should all be on one line, and follow this pattern (that includes emails): Copyright 2016 Author Name ([email protected])
@felipesanches Okay, after reviewing https://github.com/felipesanches/BevanFont/blob/8d12a34057f947468575250d3f40f3d3ece61d46/1.0/fontbakery_check_results.md I think all the fb.error() methods need to telling people what to change in order to fix the problem in their source files.
Where we have an autofix, this should be easy for you to write.
Where we don't, or you're not sure, make a FIXMEDC and I can then run through the file and make a single PR that fixes them all
Offline Felipe said,
[output could] read like a human-language report saying things like:
You need to add proper licensing info to the NAME table and make sure you use '(c)' instead of © [...]
and so on, with the rationale of each fix that needs to be done. This is specially important for those things that are impossible for us to automatically hotfix because they require human input.
I agree
I think it makes more sense for me to tackle this, so I'm going to assign it to me. @felipesanches please address the other issues :)
I've been constantly improving the messages. It will still be good to have @davelab6 review and maybe further improve them. But that's definitely not as critical as it was before, so I'm leaving it assigned to 2.0.0 milestone.
I think this must be split into a few individual issues.
I decided that it is worthless to work on the docstrings. The other kinds of metadata may be good to have but I think it is not a high-priority task. I'd rather work on other things. Even a proper testsuite is something more critical than this I think. Let's drop the docstring task and deprioritize the additional metadata features ("what to do to fix it in source files" / "links to exemplary valid fonts/projects").
reassigning to @m4rc1e and moving to 0.4.x because this relates to sources; the main thing is to go over all check strings and make sure that they will make sense for our designers to fix issues themselves :)
@davelab6 I've added this to gf-glyph-scripts. I'd far rather they were able to see and get it right in the source file.
As you know, I wrote a custom logger for them which has more detailed explanations of the issues.
I'd prefer it if FB had details of checks in the doc strings and less verbose error messages. If we need further info on an issue, we can read the code/doc strings.
Also, the fix script in gf-glyphs-scripts will fix 90% of all the errors automatically. I will try and tackle the other 10% with my assistant next week.
The only other thing I can think of which will make this more designer friendly is an improvement upon the gf-doc, Quick Start Glyphs, or should I write a new doc on QA/Production?
My plan for fontbakery so far is:
In the docstring of a test
- all text before the first empty line is a short description.
- all text after the first empty line is the documentation with details. I suggest to mardown-format this. Thus:
@test(
id='com.example.tests/test/123'
)
def my_example_test():
"""This is a short description of the test. Make
sure your font passes it.
# This can be markdown formatted user facing documentation
* make sure your software is up to date
* are there any syntax errors
* did you try turning it off and on again
"""
yield PASS, "Always good."
Another idea is to add an optional parameter to the decorator to store reference links:
@test(
id='com.example.tests/test/42'
, links=[
('the problem', 'http://typedrawers.com/discussion/228/cyrillic-kern-strings')
, ('the answer', 'https://github.com/googlefonts/fontbakery/issues/681#issuecomment-320230786')
]
)
def my_example_test():
pass
And finally, I'd like to combine all this information and auto generate an online help on the dashbord site. So we could just add links to the error output and send the designers directly to the documentation page. No need to read the source, Luke :-)
No one will read source code 😂
On Aug 4, 2017 8:28 AM, "Lasse Fister" [email protected] wrote:
My plan for fontbakery so far is:
In the docstring of a test
- all text before the first empty line is a short description.
- all text after the first empty line is the documentation with details. I suggest to mardown-format this. Thus:
@test( id='com.example.tests/test/123' )def my_example_test(): """This is a short description of the test. Make sure your font passes it. # This can be markdown formatted user facing documentation * make sure your software is up to date * are there any syntax errors * did you try turning it off and on again """ yield PASS, "Always good."
Another idea is to add an optional parameter to the decorator to store reference links:
id='com.example.tests/test/42' , links=[ ('the problem', 'http://typedrawers.com/discussion/228/cyrillic-kern-strings') , ('the answer', 'https://github.com/googlefonts/fontbakery/issues/681#issuecomment-320230786') ] )def my_example_test(): pass And finally, I'd like to combine all this information and auto generate an online help on the dashbord site. So we could just add links to the error output and send the designers directly to the documentation page. No need to read the source, Luke :-) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/googlefonts/fontbakery/issues/681#issuecomment-320236255>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAP9yzx4FqiUozhZwAGShNJU8KMLbbQnks5sUw5lgaJpZM4HbYPN> .
Yes! I don't like over stuffing the decorator though
On Aug 4, 2017 9:03 AM, "Lasse Fister" [email protected] wrote:
No one will read source code 😂
So you like what I proposed?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/googlefonts/fontbakery/issues/681#issuecomment-320243186, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP9y1-89s_uMu4j6xIXfKHUBAhsUinfks5sUxaggaJpZM4HbYPN .
It looks ugly, and overly complicated. Docstrings looks good and are simpler.
On Aug 7, 2017 6:11 AM, "Lasse Fister" [email protected] wrote:
I don't like over stuffing the decorator though
Really, why?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/googlefonts/fontbakery/issues/681#issuecomment-320625155, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP9y4ob-oTXH93oijfhvrVEMQuErdZKks5sVuLVgaJpZM4HbYPN .
Decorator bikeshedding aside, how is this looking, @graphicore and @m4rc1e ?
@m4rc1e do you have docstrings done nicely for gf-glyphs-scripts?
(The state of decorator bikeshedding: There are a lot of checks with (undocumented #1584) keyword arguments like rationale request (a link to an issue on gh usually) etc.)
I still think we should have tooling for both, the command line an the dashboard. Where the dashboard could also serve as a more complete tool to also discuss checks and their related information (we don't want to have big discussions in the source code/docstrings). A faster way could be to have some kind of permanent git issues? Anyways, we started asking people to use FB and build their own specs, thus they will have to review our checks. Having a browser based interface but also just a CLI will set the bar lower than looking into the sources. (Why are people so afraid of source code?). And, for future contributions, we'd have a central place to discuss and document our library of checks.
I'd suggest to start with a very simple CLI and then maybe plan and do a good, minimal web interface that can be extended in the future.
I would avoid storing important info on gh issues. The source repository is where important info should be so that we don't loose such info when/if github goes down. Keeping the important stuff in the code base itself makes it more future-proof.
Having said that, I do acknowledge that we sometimes rely on github as a place to look for specific info (such as metadata like request=gh-issue-url)