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

dsl support for stringjoin struct repr and stringprefix union repr #397

Merged
merged 2 commits into from
Apr 5, 2022

Conversation

ericevenchick
Copy link
Contributor

Adds support for representation stringjoin for structs and representation stringprefix for unions.

@rvagg
Copy link
Member

rvagg commented Apr 4, 2022

Nice work @ericevenchick! would you mind taking a look at how the tests for these are working and see if you can figure out how to pull those in? They should be using fixtures that are coming in from the ipld/ipld repo, but those fixtures may need to be touched up to make them work (the fixtures are based on some older code but things have changed a bit since them, including the way we consume them).

@ericevenchick
Copy link
Contributor Author

Hey @rvagg! I actually do have those test fixtures working and just pushed to https://github.com/ericevenchick/ipld. Since it's a submodule, I'm not sure of the best way to go about the PR process.

Should I go ahead and create a PR on the ipld repo, get that merged, then update the submodule on this PR?

@warpfork
Copy link
Collaborator

warpfork commented Apr 5, 2022

I went ahead and reviewed and merged that fixture PR -- you're unstuck now :) (I do admit, the process of PR'ing something with a submodule bump when you can't push to the existing URL is pretty gnarly, and I hadn't given that a lot of thought before...)

Pretty sure the rest of this LGTM too after the bump to include the new test fixtures.

@warpfork
Copy link
Collaborator

warpfork commented Apr 5, 2022

Huzzah, new tests, and green! I think I can merge this then.

Thank you!! It's wonderful to have help making this more system complete!

@warpfork warpfork merged commit 0f76b5f into ipld:master Apr 5, 2022
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.

3 participants