Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

packager should tell the user in its CLI output that a reference to Article is missing from METADATA.pb #1010

Open
n8willis opened this issue Aug 1, 2024 · 0 comments

Comments

@n8willis
Copy link

n8willis commented Aug 1, 2024

Background: Related to google/fonts#7875, if the METADATA.pb file in a PR doesn't have a files {} stanza pointing to the ARTICLE.en_us.html source file, the packager command creates an empty one. The empty one then triggers a FAIL in Fontbakery that is obtuse for the user, because it complains about a missing HTML tag in the ARTICLE.en_us.html file (and the HTML tag in the Fontbakery report is un-escaped, so it's invisible in the message that it posts to the GitHub report).

The underlying problem there is that the file isn't referenced in METADATA.pb; the packager could report that problem to the user on the command line, like it does for other serious warnings before it generates the PR. Then the user could fix the central issue immediately.

This (line four) is the message printed by packager when it generates the empty ARTICLE file because it doesn't see it in METADATA.pb:

gftools-ARTICLE-run-messages-article-created

That "Wrote" line does not look like an error. Especially because the ARTICLE.en_us.html file is in the repo. Compare to:
gftools-ARTICLE-run-messages-warning

It might just require tweaking the output. The message printed would be more helpful if it was aligned to the others, with a timestamp and so on (because it kind of looks like it's line-wrapping overflow, rather than a message), and IMO it should use highlighting a la "Warning". But if the empty file is going to cause Fontbakery to fail, I think it should provide a clearer message, telling the user that it was not finding the file {} entry in the METADATA.pb file, rather than just reporting the presence/creation of the file -- because editing the entry in METADATA.pb is the fix it needs.

(For completion, the message printed when the ARTICLE file is found via the METADATA.pb file{} is also not lined up with the other CLI stdout output; changing that too might also help a tad, since users would grow accustomed to seeing it:
gftools-ARTICLE-run-messages-article-exists

)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant