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

Put clarifying language in migration guide regarding the serde:: and non-serde:: paths #715

Merged
merged 2 commits into from
Aug 7, 2024

Conversation

mcclure
Copy link
Contributor

@mcclure mcclure commented May 28, 2024

This past week I ported a crate from bincode1 to bincode2. I had some difficulty because I took the "migrate serde to bincode-derive" path, which was not appropriate for my application. I was following the migration guide, but I misread it.

I would like to recommend some minor language tweaks to make the migration guide more resistant to misinterpretation. Proposed language is attached, but I don't care if you use my specific language. I do suggest you make some form of the following changes:

  • Make it clearer that the "Migrating with serde" and "Migrating to bincode-derive" sections describe exclusive options.
  • Describe why someone might want to intentionally migrate with serde.

This would have helped me do my porting correctly the first time.

docs/migration_guide.md Outdated Show resolved Hide resolved
Copy link

stale bot commented Jul 27, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 27, 2024
@mcclure
Copy link
Contributor Author

mcclure commented Aug 2, 2024

Nothing has changed, so this problem is still real. I still think this patch should be merged.

@stale stale bot removed the stale label Aug 2, 2024
@VictorKoenders
Copy link
Contributor

Please fix the change I've requested and we can get this merged

@mcclure
Copy link
Contributor Author

mcclure commented Aug 3, 2024

Sorry, I misunderstood your comment before. Is that better?

@VictorKoenders VictorKoenders merged commit d3011fe into bincode-org:trunk Aug 7, 2024
68 of 69 checks passed
@VictorKoenders
Copy link
Contributor

Thanks!

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