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

Fix fontbakery installation and add support for fontbakery extras dependencies installation #211

Merged
merged 17 commits into from
Oct 13, 2023

Conversation

chrissimpkins
Copy link
Contributor

@chrissimpkins chrissimpkins commented Oct 13, 2023

Supersedes #210.

Includes all of @madig's commits in that branch

Closes #165

@chrissimpkins
Copy link
Contributor Author

Will address the comment in #210 (comment) in this branch

@chrissimpkins chrissimpkins linked an issue Oct 13, 2023 that may be closed by this pull request
@chrissimpkins chrissimpkins changed the title Fix fontbakery installation Fix fontbakery installation and add support for fontbakery extras dependencies installation Oct 13, 2023
@chrissimpkins
Copy link
Contributor Author

chrissimpkins commented Oct 13, 2023

@felipesanches mind having a look at the error with fontbakery execution in the test https://github.com/f-actions/font-bakery/actions/runs/6511875234/job/17688387802?pr=211? We run CI testing with fontbakery on this GHA implementation and it uses the custom check profile defined in:

https://github.com/f-actions/font-bakery/blob/master/tests/check-test.py

It looks like the custom fontbakery profile approach may need to be updated with recent developments in fontbakery?


Edit: spun this out into a new issue in #212. We'll likely merge this PR before we address it

@chrissimpkins
Copy link
Contributor Author

More than willing to approach testing execution of FB through this Action differently if you have a better way to test.

@chrissimpkins
Copy link
Contributor Author

chrissimpkins commented Oct 13, 2023

I reviewed Niko's new fontbakery installation logic and I am ok with the new backwards incompatible approach here released under a new major release version of this Action. We can ask any users who happen to need a fontbakery version < 0.9.0 (0.9.0 released last month) to stick with the v2 branch of this Action. Users should be pinning their Action execution to a v1, v2, v3 (upcoming for these changes) major release anyways. If they happen to be tracking main branch commits on the Action and are installing configured fixed versions of fontbakery, this is a situation where we break them but I think that this is an inappropriate use of the Action and we'll document around it so that they understand how to address the issue. I'm good with it. Thanks for the changes Nikolaus!

@madig @felipesanches

@chrissimpkins
Copy link
Contributor Author

we'll document around it

c20d1b3

@chrissimpkins
Copy link
Contributor Author

chrissimpkins commented Oct 13, 2023

Note to self: I added a backwards incompatible change to the path input with the changes here. path will become a required input with no fallback default. Documentation updated in c20d1b3

@chrissimpkins
Copy link
Contributor Author

Tested execution of this branch at c20d1b3 on a font development project that uses the Action in its CI pipeline. Execution took place without issues. I'm going to merge and cut the v3.0.0 release.

Thanks to Nikolaus and Felipe for their help here!

@chrissimpkins chrissimpkins merged commit 5d1d717 into master Oct 13, 2023
@chrissimpkins chrissimpkins deleted the madig-fix-installs branch October 13, 2023 20:07
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

Successfully merging this pull request may close these issues.

How can we specify extras?
2 participants