Skip to content
New issue

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

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

Already on GitHub? # to your account

qa: return errcode 1 if fontbakery reports a fatal error #800

Merged
merged 2 commits into from
Jan 3, 2024
Merged

Conversation

m4rc1e
Copy link
Collaborator

@m4rc1e m4rc1e commented Dec 14, 2023

Will merge once a new fontbakery release is cut. PR implementation will simply log a fatal error and stop all other checks from running.

cc @simoncozens @felipesanches

@m4rc1e m4rc1e requested a review from simoncozens December 14, 2023 10:28
@m4rc1e

This comment was marked as outdated.

@simoncozens
Copy link
Contributor

LGTM. Please remember that .ci/run.py in google/fonts will also need check=True on its subprocess.run calls to propagate the error code.

@@ -131,6 +133,10 @@ def fontbakery(self, profile="googlefonts", html=False, extra_args=None):
msg = doc.read()
self.post_to_github(msg)

if process.returncode != 0:
logger.fatal("Fontbakery has raised a fatal error. Please fix!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, my goal is to gradually change the threshold in this CI setup. So, instead of saying "raised a fatal error", this could say: "There are check results at the current threshold: FATAL"

And in the future that would become something like:
"There are check results at (or worse than) the current threshold: FAIL severity 10"

@m4rc1e m4rc1e merged commit 91939f9 into main Jan 3, 2024
11 checks passed
@felipesanches
Copy link
Member

thanks! ;-)

@simoncozens simoncozens deleted the qa-err-code branch May 30, 2024 07:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants