Skip to content

Conversation

@bjosv
Copy link
Collaborator

@bjosv bjosv commented Dec 18, 2024

Remove valkeyAsyncSetConnectCallbackNC since it was first added in hiredis for backwards-compatibility reasons,
and let valkeyAsyncSetConnectCallback use the non-const connect callback prototype.

Includes the removal of the cluster version valkeyClusterAsyncSetConnectCallbackNC and updates of tests.

Signed-off-by: Björn Svensson <[email protected]>
@zuiderkwast
Copy link
Collaborator

There's a typo in redisClutserAsyncSetConnectCallback.

@bjosv
Copy link
Collaborator Author

bjosv commented Dec 19, 2024

Well spotted, seems like the spellchecker in CI work quite fine as well.

Signed-off-by: Björn Svensson <[email protected]>
Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@michael-grunder michael-grunder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The original weirdness was a bit of a hack.

@bjosv bjosv merged commit ca3dac9 into valkey-io:main Dec 30, 2024
43 checks passed
@bjosv bjosv deleted the connect-cb branch December 30, 2024 12:17
bjosv added a commit to bjosv/libvalkey that referenced this pull request Apr 28, 2025
Some examples were missed when changing the connect callback
signature in PR valkey-io#142 to using a non-const context.

Add -Werror to the examples Makefile to find issues in CI.
Can be disabled by USE_WERROR=0 as when building libvalkey.

Signed-off-by: Björn Svensson <[email protected]>
bjosv added a commit that referenced this pull request Apr 29, 2025
Some examples were missed when changing the connect callback
signature in PR #142 to using a non-const context.

Add -Werror to the examples Makefile to find issues in CI.
Can be disabled by USE_WERROR=0 as when building libvalkey.

Signed-off-by: Björn Svensson <[email protected]>
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