-
Notifications
You must be signed in to change notification settings - Fork 41
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
Remove the legacy codec system #164
Comments
Agreed. Though I'd like to get close to feature parity, so that people using the legacy codec can smoothly upgrade. |
Heya, @vmx asked me to comment on our usage of the codec approach. To be honest, my first thought was the same as yours @Stebalien but there are two things that I enjoy in the codec approach:
I would love for there to be one true way, and if serializing data as blocks was as simple as deriving |
I agree that we definitely need a way to cleanly extract links. It shouldn't be too difficult to re-implement that with serde by just throwing away everything we don't care about, but the current method is more efficient. |
I recently learned about the |
That's not even necessary, really. We can just deserialize into a "links collector" that:
|
Ah right, that should be possible too. When working with many small blocks, the current API allows to collect all links into a single container (without allocating a links container for each block), that's why I thought of Maybe there's two features at play even?
The latter might not matter much in practice though. |
Agreed. I'd start with the former one and add the latter only if it turns out to be useful. I prefer keeping APIs small and targeted at needs, rather then adding theoretically useful things. |
Serde is significantly more usable, and supporting two codec systems is very confusing and a waste of time.
The text was updated successfully, but these errors were encountered: