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

Fix existing bug with multibase ToCid #167

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Fix existing bug with multibase ToCid #167

merged 3 commits into from
Jan 16, 2020

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jan 15, 2020

  • Refactored and added tests for existing ToCid code block. (I want to do a complete rewrite of this in the future, since this trait is not necessary and isn't well setup imo or tested, but not a priority)
  • Added string decoding test to make sure version and codec we are using will be decodable when needed

decoding from string to bytes is broken currently because multibase 0.6 is broken. When updating to 0.7 they removed at least lower case base32 so I wrote a test that currently panics because it isn't supported (but just throws a parse error lol) and will either open an issue or add support for other encoding types in multibase and update at a later time.

There is no rush on this because we will be interacting with bytes, but will document now for when it gets to that point.

Edit: The rewrite I'm referencing by the way for ToCid is to just use the TryFrom trait as this is exactly the same as the trait they setup and is more usable: https://doc.rust-lang.org/std/convert/trait.TryFrom.html

@austinabell austinabell merged commit e70250d into master Jan 16, 2020
@austinabell austinabell deleted the austin/tocidfix branch January 16, 2020 22:14
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.

5 participants