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

[chore] Update write-fonts to 0.21.0 #665

Merged
merged 1 commit into from
Dec 16, 2023
Merged

[chore] Update write-fonts to 0.21.0 #665

merged 1 commit into from
Dec 16, 2023

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Dec 16, 2023

This involves a breaking change to the VariationStoreBuilder, which in turn requires us to tweak the FeatureProvider trait in fea-rs, but overall fairly painless.

This involves a breaking change to the VariationStoreBuilder, which in
turn requires us to tweak the FeatureProvider trait in fea-rs, but
overall fairly painless.
@@ -233,7 +233,7 @@ mod tests {
}

fn build_exactly_one_subtable(self) -> PairPos {
let mut varstore = VariationStoreBuilder::new();
let mut varstore = VariationStoreBuilder::new(0);
Copy link
Member

Choose a reason for hiding this comment

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

how about we add a VariationStoreBuilder::nop() alternative constructor that would set axis_count = 0? It doesn't make much sense for the actual new() default constructor to take an axis_count that is equal to 0, because there's nothing to "build" in that case, the varstore won't contain any regions or deltas if there are no axes...

Copy link
Member Author

Choose a reason for hiding this comment

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

given that this should only ever be needed in tests I'm not too fussed either way? That we need this at all is mostly just indicates there's something smelly in our API (although there's a rationale for that)

@anthrotype anthrotype added this pull request to the merge queue Dec 16, 2023
Merged via the queue into main with commit 32ebd5d Dec 16, 2023
10 checks passed
@anthrotype anthrotype deleted the update-write-fonts branch December 16, 2023 15:20
@cmyr cmyr mentioned this pull request Dec 18, 2023
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.

2 participants