search

alexheretic / ab-glyph

Rust API for loading, scaling, positioning and rasterizing OpenType font glyphs
Apache License 2.0
372 stars 24 forks source link

Adding new methods to `Font` are breaking changes #97

Closed Rodrigodd closed 8 months ago

Rodrigodd commented 8 months ago

Hello, I have an unpublished crate that contains an implementation of the trait ab_glyph::Font. In PR #88 a new method was added to Font which broke my crate. This has also happened before, with PR #35 I believe.

Looking around the issues and the release cadence, I see that you are very mindful about breaking changes, so I am making this report only in case you are not aware of this one. My crate is only used for personal projects, so it does not have that much of an impact for me to mind too much.

alexheretic commented 8 months ago

Yes, thanks for pointing this out. I made a mistake merging them without considering external Font impls.

Probably these methods should have been introduced in new traits instead.

I think at this point too much time has passed to yank stuff and address these particular breaks. Or is there a particular action you have in mind?

If not, I'll simply try to avoid repeating this mistake. Please let me know about any future breaks you see.

Rodrigodd commented 8 months ago

Or is there a particular action you have in mind?

Nothing in particular. But I imagine you may want to make this trait a sealed trait in a future minor/major version. My use case at least is purely for convenience as far as I remember, so it would not affect me.

Searching on GitHub, I found two implementations of the trait: in Ryan1729/rote and LechintanTudor/anchor. In both cases, they are just implementations for a Font wrapper, like mine. I don't know if there is any other compelling use case for a user-defined implementation.

In any case, I will close this issue, as I imagine there is nothing that needs to be addressed immediately.