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

WIP: Use string as the internal representation and allow an associated multibase. #65

Closed
wants to merge 6 commits into from

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Aug 5, 2018

This is an alternative to #64 to explore the design space.

The main advantage of this is that it does not break any existing code.

This might also perform a little better, but I image it will be negligible.

@ghost ghost assigned kevina Aug 5, 2018
@ghost ghost added the status/in-progress In progress label Aug 5, 2018
@kevina kevina changed the title RFC: Use string as the internal representation and allow an associated multibase. WIP: Use string as the internal representation and allow an associated multibase. Aug 6, 2018
@kevina kevina mentioned this pull request Aug 24, 2018
@Stebalien
Copy link
Member

As I've stated before, mixing representation with data is almost never something one should do.

  1. We won't be able to use CIDs directly in maps. Being able to iterate over a map with CIDs and values would be really nice. If users do do this, they'll lose the multibase information.
  2. Not all representations will be able to conserve the multibase. If the user gives me a set of CIDs and I pack them into an IPLD object, they'll lose the base.
  3. It's fixing a UX problem by threading display information throughout the program. From a design standpoint, that's just icky.

@kevina
Copy link
Contributor Author

kevina commented Aug 25, 2018

@Stebalien both (1) and (2) are fine. If the information is lost that's okay the idea was to preserve it when possible.

For (3) we will have to agree to disagree. As I see it it is threading through optional metadata that just "goes along for the ride".

However, I can see how this add complexity and don't plan to push it.

@kevina
Copy link
Contributor Author

kevina commented Aug 30, 2018

Closing, as we are going with a pure string representation. Please don't delete the branch.

@kevina kevina closed this Aug 30, 2018
@ghost ghost removed the status/in-progress In progress label Aug 30, 2018
@mvdan mvdan deleted the kevina/interface-alt branch July 1, 2021 16:13
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