-
Notifications
You must be signed in to change notification settings - Fork 15
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
CI to expose build instability #656
Conversation
2dbdbd9
to
9354b88
Compare
Rebased on main |
I believe it's still going to fail because of varying GSUB FeatureParams nameIDs |
how about instead of just calling ttx -l you actually diff the output for two ttx -l calls? So it only shows what changed which is what we are interested in $ diff -u <(pipx run fonttools ttx -l first-roman.ttf) <(pipx run fonttools ttx -l second-roman.ttf)
$ diff -u <(pipx run fonttools ttx -l first-italic.ttf) <(pipx run fonttools ttx -l second-italic.ttf) |
That's right. I don't think we can merge it quite yet. |
just rebased on main hoping that things just work |
looks like gvar and head are still reporting same lengths but different checksums https://github.com/googlefonts/fontc/actions/runs/7251757185/job/19754763563#step:12:19 ![]() |
in order to have it exported also for the subsequent steps https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-environment-variable
Guess we can't declare victory yet. Even so, I'm quite pleased GSUB is now fixed :) |
After googlefonts/fontations#750 we can temporarily declare victory ✌️ |
with latest write-fonts 0.21.1, CI is now green! |
shall I revert this until we figure out #647 (comment)? otherwise all new PRs are going to fail because of it.. |
I'd vote not, if CI is failing we shouldn't be merging much of anything except a fix and we still have the power to merge despite this failure if we really really want to |
Expose #647. This check will fail until we fix that. I also moved build GS to be part of CI because in my mind it is.