-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Implement client flag data model. #12
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
Conversation
libs/common/tests/value_test.cpp
Outdated
|
|
||
| #include "value.hpp" | ||
|
|
||
| #include <boost/json/src.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the other PR that uses JSON is merged I will consolidate these. How about lib.cpp?
| * An object describing the main factor that influenced the flag evaluation | ||
| * value. | ||
| */ | ||
| [[nodiscard]] std::optional<std::reference_wrapper<EvaluationReason const>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really like the standard to allow const& optionals. 🙄
| return {std::string(kind), error_kind, rule_index, rule_id, | ||
| prerequisite_key, in_experiment, big_segment_status}; | ||
| } | ||
| return {"ERROR", std::nullopt, 0, std::nullopt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I am not sure how we want error handling to work in regards to value conversion. Right now I've gone with the least freaking out method. But using the built-in mechanisms for tag_invoke doesn't give us the greatest methods for communicating error details. Unless I encode parsing errors into the object. Or we parse into something like the tl::expected, and then check that result.
| if (json_value.is_object()) { | ||
| auto json_obj = json_value.as_object(); | ||
|
|
||
| auto* kind_iter = json_obj.find("kind"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most samples use at, but I would really like it to not explode into fire.
| } | ||
|
|
||
| TEST(EvaluationResultTests, FromMapOfResults) { | ||
| auto results = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically what we would be doing for a PUT event.
|
We will need another type to handle patches. Basically an EvaluationResult which also has a key. I think we can do that in the client specific code though. So I will do it independent of this PR. |
| * should send debug events for the flag. | ||
| * @return | ||
| */ | ||
| [[nodiscard]] std::optional<uint64_t> debug_events_until_date() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could maybe use something like std::chrono::time_point<std::chrono::system_clock> or whatever to make this a bit more clear. If that even makes sense here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will look at it.
Co-authored-by: Casey Waldren <cwaldren@launchdarkly.com>
Co-authored-by: Casey Waldren <cwaldren@launchdarkly.com>
|
|
||
| #include <boost/json.hpp> | ||
|
|
||
| namespace launchdarkly { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only added specializations for types I am using. Then added the generic mapper for going from one of those types to some non-default type.
We don't use many types, but I will probably add some in future PRs? Maybe.
No description provided.