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

zig fmt + some fixes for Zig 0.11 release #7

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

dustyrockpyle
Copy link
Contributor

I have a personal project I wanted to update to 0.11 that depends on protobuf-zig - thought I would send out a PR updating this library to Zig 0.11 in case it saves you some time.

I just ran zig fmt and then fixed up 3 or 4 alignment issues that were showing up in the tests on free. Some of the @as from zig fmt could probably be cleaned up.

All tests pass without issue on my machine

@travisstaloch travisstaloch merged commit 4c472e2 into travisstaloch:main Sep 4, 2023
1 check passed
@travisstaloch
Copy link
Owner

then fixed up 3 or 4 alignment issues that were showing up in the tests on free. Some of the @as from zig fmt could probably be cleaned up.

I had been seeing the same alignment on free problems and couldn't figure out how to fix them. Much appreciated! 👍

@travisstaloch
Copy link
Owner

@dustyrockpyle would you mind leaving a comment explaining how you fixed the free alignment issue either here or in the 'files changed' tab? Its just to help me understand how you fixed the problem. Perhaps it was related to PtrAlignCast()? I just couldn't easily spot what changed looking through the changes.

@dustyrockpyle
Copy link
Contributor Author

#8

Yeah probably should have done it this way to begin with, there's a better commit layout.

I might have missed some also - I only fixed what was showing up as errors in the tests. In my own code I believe I'm only reading protobufs from a buffer, so I never deinit them.

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