Skip to content

Conversation

kinyoklion
Copy link
Member

No description provided.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #204435: C bindings for flag notifier.


// This test registers a listener. It doesn't use the listener, but it
// will at least ensure 1.) Compilation, and 2.) Allow sanitizers to run.
TEST(ClientBindings, RegisterFlagListener) {
Copy link
Member Author

Choose a reason for hiding this comment

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

When we do TestData for the server possibly we can add something similar to the client. It would make legitimate testing of these much simpler.

On the C++ side it is better tested because we can directly make the updater.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did test this a number of ways in the hello-c app.

#pragma once

namespace launchdarkly::client_side {
namespace launchdarkly {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was moved to common, but client_side was not removed.

LD_ASSERT_NOT_NULL(sdk);
LD_ASSERT_NOT_NULL(flag_key);

auto connection = TO_SDK(sdk)->FlagNotifier().OnFlagChange(
Copy link
Contributor

Choose a reason for hiding this comment

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

If listener.FlagChanged is null, then is there any need to register a callback?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could re-word the documentation to indicate that the returned connection can be NULL if no listener is registered.

Currently the need to register a listener is just to get the connection.

@kinyoklion kinyoklion requested a review from cwaldren-ld May 30, 2023 21:11
@kinyoklion kinyoklion merged commit 11a7f61 into main May 30, 2023
@kinyoklion kinyoklion deleted the rlamb/sc-204435/flag-notifier-bindings branch May 30, 2023 23:12
@github-actions github-actions bot mentioned this pull request May 30, 2023
cwaldren-ld pushed a commit that referenced this pull request May 31, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>launchdarkly-cpp-client: 0.2.0</summary>

##
[0.2.0](launchdarkly-cpp-client-v0.1.0...launchdarkly-cpp-client-v0.2.0)
(2023-05-31)


### Features

* add AllFlags C binding
([#128](#128))
([9aa0794](9aa0794))
* Add C bindings for data source status.
([#124](#124))
([d175abb](d175abb))
* Add c bindings for FlagNotifier.
([#119](#119))
([11a7f61](11a7f61))
* add Version method to obtain SDK version
([#122](#122))
([1003117](1003117))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-internal bumped from 0.1.0 to 0.1.1
    * launchdarkly-cpp-common bumped from 0.1.0 to 0.2.0
</details>

<details><summary>launchdarkly-cpp-common: 0.2.0</summary>

##
[0.2.0](launchdarkly-cpp-common-v0.1.0...launchdarkly-cpp-common-v0.2.0)
(2023-05-31)


### Features

* add AllFlags C binding
([#128](#128))
([9aa0794](9aa0794))
* Add C bindings for data source status.
([#124](#124))
([d175abb](d175abb))
* Add c bindings for FlagNotifier.
([#119](#119))
([11a7f61](11a7f61))
* Allow for easier creation of contexts from existing contexts.
([#130](#130))
([5e18616](5e18616))


### Bug Fixes

* rename C iterator bindings to follow new/free pattern
([#129](#129))
([24dff9a](24dff9a))
</details>

<details><summary>launchdarkly-cpp-internal: 0.1.1</summary>

### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-common bumped from 0.1.0 to 0.2.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This was referenced May 31, 2023
@github-actions github-actions bot mentioned this pull request May 13, 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

Successfully merging this pull request may close these issues.

2 participants