com.google.fonts/check/mac_style: Skip if font.style is None

[khaledhosny]

If font.style is None, the test currently errors.

Related to #4349, but not a complete fix.

(Provide a list of all the noteworthy changes you've done. Elaborate on any decisions you had to make, and include links and/or screenshots, if applicable)

Checklist

• update CHANGELOG.md
• wait for the tests to pass
• request a review

[simoncozens]

An alternative way to do this is to make "style" a condition, which would make it automatically skipped if it returned a false value. I'm not necessarily suggesting you do this, but I'd be interested to know why you didn't:

1. didn't know about it
2. did know about it, but found it non-obvious/unidiomatic
3. something else

And if (1), I'd be interested what you think now you do know about it. Basically I'm trying to listen more to check implementors to get a handle on how they see and want to use the facilities the checkrunner has to offer.

[khaledhosny]

An alternative way to do this is to make "style" a condition, which would make it automatically skipped if it returned a false value. I'm not necessarily suggesting you do this, but I'd be interested to know why you didn't:

1. didn't know about it

2. did know about it, but found it non-obvious/unidiomatic

3. something else

And if (1), I'd be interested what you think now you do know about it. Basically I'm trying to listen more to check implementors to get a handle on how they see and want to use the facilities the checkrunner has to offer.

I knew about conditions, but I didn’t know how they work, and it didn’t occur to me that any Font attribute can be used as a condition. I see now this is documented in https://github.com/fonttools/fontbakery/blob/main/docs/source/developer/writing-profiles.md but https://font-bakery.readthedocs.io/en/latest/developer/writing-profiles.html still has the old documentation.

[simoncozens]

Thanks. Yeah, we're moving from font-bakery to fontbakery as we're not sure who set up the old docs but they did it with an unusual naming scheme... ;-)

[khaledhosny]

Any remaining issues with this PR?

[simoncozens]

LGTM

[felipesanches]

It is a simple PR, but it degraded the pre-existing code-tests. I will address this later this afternoon.

[khaledhosny]

It is a simple PR, but it degraded the pre-existing code-tests. I will address this later this afternoon.

I don’t understand how my change degrades anything, but I drop that refactoring commit, it is not needed for anything.

[felipesanches]

It is a simple PR, but it degraded the pre-existing code-tests. I will address this later this afternoon.

I don’t understand how my change degrades anything, but I drop that refactoring commit, it is not needed for anything.

The reason why it was bad before was because, in order to check the new SKIP case, it was not checking the keywords of the other cases anymore, thus making the testing of the other cases less effective. But now that you dropped that commit, it is looking good! Thanks