fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
534 stars 99 forks source link

Make is_bold condition work with more style naming formats #4668

Closed arrowtype closed 2 months ago

arrowtype commented 2 months ago

Description

Relates to issue https://github.com/fonttools/fontbakery/issues/4667

Changes:

If this is an acceptable change, I can update the changelog, etc. I’m partly submitting this so I can present a proposed solution, rather than just a complaint.

Checklist

felipesanches commented 2 months ago

yes, this is an acceptable change ;-)

Thanks!

felipesanches commented 2 months ago

Be sure to run black on your code:

Screenshot from 2024-04-23 20-51-06

arrowtype commented 2 months ago

Hey @felipesanches, thanks so much for the review! Ha, don’t know how I managed to double up on the .lower(), but I’ve corrected that and formatted with Black. I’ve also attempted to update the changelog, but I’m not 100% sure of the best message to include there. Please feel free to adjust that as you see fit!

felipesanches commented 2 months ago

Great! I'll take a look, thanks

felipesanches commented 2 months ago

Even though the condition is globally available, the is_bold condition is currently only used in the implementation of com.fontwerk/check/style_linking, which is included in both FontWerk and Type Network profiles.

Are you using one of these profiles? Or do you plan to use is_bold on some other check?

felipesanches commented 2 months ago

I see that at https://github.com/fonttools/fontbakery/issues/4667#issue-2259756279 you mentioned com.fontwerk/check/style_linking explicitly, so I will move on with the code-review to merge this.

arrowtype commented 2 months ago

Awesome, thanks for your review and all your help here, @felipesanches!