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

Switch to opaque structs #39

Open
michael-grunder opened this issue Jun 29, 2024 · 3 comments
Open

Switch to opaque structs #39

michael-grunder opened this issue Jun 29, 2024 · 3 comments
Assignees

Comments

@michael-grunder
Copy link
Collaborator

Right now, we declare valkeyContext, valkeyAsyncContext, valkeyClusterContext, and valkeyAsyncClusterContext in our public headers, exposing their memory layout.

This makes it a lot harder to make changes to them without breaking the ABI contract and therefore having to bump the SONAME.

We could switch to simply defining:

typedef struct valkeyContext valkeyContext;
typedef struct valkeyClusterContext valkeyClusterContext;

in our public headers, and declare the actual struct internally in a private header.

The only real downside is that it might break code for users who are directly accessing struct members. Personally, I can live with that 😄.

@bjosv @zuiderkwast What does everybody think?

@zuiderkwast
Copy link
Collaborator

Björn is on vacation now. I say go for it!

Users have to migrate to this project anyway, at least the renaming parts. There are other changes too. We should collect all these changes for release notes and migration guide.

@zuiderkwast
Copy link
Collaborator

@bjosv says OK. If some user needs access to some field, we can add accessor functions for those later on.

@bjosv
Copy link
Collaborator

bjosv commented Jul 1, 2024

Yes, I like the idea. Lets add accessors when/if the time comes.

@michael-grunder michael-grunder self-assigned this Jul 2, 2024
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

No branches or pull requests

3 participants