Skip to content

Conversation

@cwaldren-ld
Copy link
Contributor

@cwaldren-ld cwaldren-ld commented Aug 24, 2023

This implements a set of C bindings for the server-side SDK.

Missing are flag notifier bindings, but this isn't implemented in the C++ side yet either.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #210041: SDK Client C bindings.

@cwaldren-ld cwaldren-ld changed the base branch from main to server-side August 24, 2023 23:16
@cwaldren-ld cwaldren-ld force-pushed the cw/sc-210041/server-client-bindings branch 8 times, most recently from b6750d8 to b5a3725 Compare August 30, 2023 22:17
@cwaldren-ld cwaldren-ld marked this pull request as ready for review August 30, 2023 22:50
@cwaldren-ld cwaldren-ld requested review from a team and kinyoklion August 30, 2023 22:50
LD_EXPORT(char*)
LDAllFlagsState_SerializeJSON(LDAllFlagsState state);

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole comment points to the danger of this kind of API.

Theoretically we could create a new typedef LDValueReference for these situations. And all the bindings. But it wouldn't have a binding for _Free. That'd prevent at least one error.

Another thing we could do is name this _ValueReference or something, rather than _Value.

/**
* Enumeration of possible data source states.
*/
enum LDServerDataSourceStatus_State {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I admit these enums are quite ugly - but needed to rename so they don't clash with the client and can be independently changed.

Welcome to suggestions..

@cwaldren-ld cwaldren-ld force-pushed the cw/sc-210041/server-client-bindings branch from a4638a2 to 409dbda Compare August 31, 2023 00:49
Adds a hello app for the server-side C bindings.

Also adds an app that includes both SDKs, to ensure they can coexist
without symbol clashes.
@cwaldren-ld cwaldren-ld merged commit 3669ff5 into server-side Aug 31, 2023
@cwaldren-ld cwaldren-ld deleted the cw/sc-210041/server-client-bindings branch August 31, 2023 23:26
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