feat: Allow for easier creation of contexts from existing contexts. #130

kinyoklion · 2023-05-31T20:52:14Z

No description provided.

kinyoklion · 2023-05-31T21:06:44Z

libs/common/tests/context_builder_test.cpp

+                       .Build();
+
+    auto builder = ContextBuilder(context);
+    if (auto updater = builder.Kind("user")) {


@mmrj Adding "state" and "sneaky" to the context.

could I also do:

updater->Set("city", "Winnemucca")

and overwrite the Reno value?

kinyoklion · 2023-05-31T21:07:02Z

libs/common/tests/context_builder_test.cpp

+                       .Set("city", "Reno")
+                       .Build();
+
+    auto updated_context =


@mmrj Adding an "org" to the context.

nice. Do I need to call IdentifyAsync with updated_context after this?

Yes, this just makes a context. You need to identify with that context to get flag values for it.

mmrj

Semantics of using this LGTM. I left a couple of questions and some nits on the comments, but nothing blocking.

mmrj · 2023-05-31T21:14:43Z

libs/common/include/launchdarkly/attributes_builder.hpp

    }

+    /**
+     * Start adding a kind to the context.


Wording nit: I think you are adding a new context with the specified kind to the builder ?

I don't know. I don't really think we are, or that if we are it isn't meaningful.

builder.Set("address", "111 Street")

I am adding an address to my context?

The address isn't an "address" though. It is a string, which I have added to my context with the name "address".

A kind is just the key for a set of attributes in a context, which we also happen to call a context.

I think we are adding a kind.

(The kind happens to be a context, like the address happens to be a string, or the name is a string.)

mmrj · 2023-05-31T21:15:24Z

libs/common/include/launchdarkly/attributes_builder.hpp

+     * Start updating an existing kind.
+     *
+     * @param kind The kind to start updating.


same as above, I think you're updating the existing context with the given kind

mmrj · 2023-05-31T21:18:21Z

libs/common/include/launchdarkly/context_builder.hpp

-     * the contents of the builder are moved into the created context.
+     * Start updating an existing kind.
     *
-     * After the context is build, then this builder, and any kind builders
-     * associated with it, should no longer be used.
+     * @param kind The kind to start updating.


same here

I think the reason I find the current wording a little confusing is because in the UI a context kind is an entity that you can create and define (Contexts list --> Kind tab --> Create kind). It's separate from any context where it's used, and this is definitely not working with that.

Yeah, adding a "org" to a context is different than adding an "org" to your launchdarky account. The "org" you put into a context has attributes, and you cannot even make it without any attributes.

Both are kinds though.

Yeah I think got hung up on the two uses of "kind":

the entity you add to your LD account, can (but don't have to) use when defining a new context in the SDK, and can get a list of using the REST API, etc.

the thing that, as you say above, happens to be a context, like the address happens to be a string, or the name is a string

mmrj · 2023-05-31T21:19:00Z

libs/common/tests/context_builder_test.cpp

+                       .Build();
+
+    auto builder = ContextBuilder(context);
+    if (auto updater = builder.Kind("user")) {


mmrj · 2023-05-31T21:19:54Z

libs/common/tests/context_builder_test.cpp

+                       .Build();
+
+    auto builder = ContextBuilder(context);
+    if (auto updater = builder.Kind("user")) {


could I also do:

updater->Set("city", "Winnemucca")

and overwrite the Reno value?

mmrj · 2023-05-31T21:20:37Z

libs/common/tests/context_builder_test.cpp

+                       .Set("city", "Reno")
+                       .Build();
+
+    auto updated_context =


nice. Do I need to call IdentifyAsync with updated_context after this?

🤖 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>

kinyoklion added 5 commits May 31, 2023 13:43

feat: Allow for easier creation of contexts from existing contexts.

d63da7d

Rename to Update.

9db82d6

Remove functional header.

8381930

Undo bad wrapping.

27cc430

Rename Update to Kind

6163d93

kinyoklion marked this pull request as ready for review May 31, 2023 21:05

kinyoklion commented May 31, 2023

View reviewed changes

cwaldren-ld approved these changes May 31, 2023

View reviewed changes

Add some consts

fb0d850

mmrj reviewed May 31, 2023

View reviewed changes

kinyoklion merged commit 5e18616 into main May 31, 2023

kinyoklion deleted the rlamb/context-ergonomics branch May 31, 2023 21:31

github-actions bot mentioned this pull request May 31, 2023

chore: release main #123

Merged

This was referenced May 31, 2023

chore: release main #133

Merged

chore: release main #134

Closed

chore: release main #136

Merged

feat: Allow for easier creation of contexts from existing contexts. #130

feat: Allow for easier creation of contexts from existing contexts. #130

Uh oh!

Conversation

kinyoklion commented May 31, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmrj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants