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

feat(core): roundtrip serde + to/from strings #2270

Merged
merged 2 commits into from Mar 16, 2023

Conversation

DaniPopes
Copy link
Collaborator

@DaniPopes DaniPopes commented Mar 16, 2023

Motivation

Refs #2038, #2268 (comment)

Doesn't fix it because you still need to replace kebab-case to snake_case

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog
  • Breaking changes

@CodeSandwich
Copy link
Contributor

From tests I wrote when adding ethlive I've found out that the last serialize parameter is the one which is used when doing to_string. In this PR you're promoting the first one, which will break a lot of downstream.

@gakonst
Copy link
Owner

gakonst commented Mar 16, 2023

Does this supersede #2268?

@DaniPopes
Copy link
Collaborator Author

From tests I wrote when adding ethlive I've found out that the last serialize parameter is the one which is used when doing to_string. In this PR you're promoting the first one, which will break a lot of downstream.

Yes. I don't consider it breaking since this is only changes the Display/ToString implementation and previous implementations can parse this "new" one because it was aliased. I also think it wasn't intended in the first place that the last item in the list is the selected one for Display.

Does this supersede #2268?

No

@gakonst gakonst merged commit 404422e into gakonst:master Mar 16, 2023
@DaniPopes DaniPopes deleted the feat/chain-roundtrip branch March 16, 2023 20:33
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.

None yet

3 participants