✨ Config v6 #133
✨ Config v6 #133
Conversation
randycoulman
force-pushed
the
adopt-styler
branch
from
fc129c9
to
31f9e44
Compare
* ♻️ Rename settings -> feature flags This is a start at encapsulating the format of the Config map. * ♻️ Introduce Config.Preferences module Refactor existing use of preferences to use new module. * ♻️ Encapsulate EvaluationFormula and rules * ♻️ Move entry matching to config modules
Always return the entire config, not just the feature flags. This will allow access to preferences and segments in the v6 config where needed.
- Add setting_type field - Rename percentage_rules -> percentage_options - Rename rollout_rules -> targeting_rules
Update constants and test literals to new format. Extract sample config to a factory.
No need to sort maps to compare them. Also, add extra keys needed after switching to use the factory config.
Begin updating the Config sub-modules to support the new Config format.
Use v2 SDK Keys and add comments with URLs pointing to the relevant dashboard
These tests require new features that are not yet implemented, so skipping for now until the support has been added.
Rename a few of the comparators and simplify descriptions to match latest Python SDK.
There was a problem hiding this comment.
In the meantime we specified a few additional tests: configcat/common-js#102
Could you add these to the Elixir SDK's test suite?
Oh, and a minor observation about the sample projects: I had to apply this workaround to be able to run the samples. (I'm on Win10). Could you maybe do something about this if it can be avoided?
Brings them into line with the main app and should also fix an issue with `ssl_verify_fun` 1.1.6.
One bug fix included: Python and JS allow whitespace around numeric values whereas Elixir does not, which broke the starts_with/ends_with hashed tests while attempting to parse the length string. We add trimming to just the length string to avoid this issue and match Python/JS functionality.
Currently failing due to change in logic in JS version; will need to investigate further.
|
The fix for the sample projects is in 1005612. I just had to update the dependencies to match what was in the main project. New tests are in 767ec96, 4c5acac, and 00b78c3. They are not quite passing yet, because the logging logic is different between the Python and JS versions and I ran out of time for tonight before I could resolve the remaining differences. I'll take another look at it tomorrow night. |
Making Elixir's standard float formatting match JavaScript's was somewhat tricky.
e.g. `%{"b" => "true"}` is invalid.
|
I've addressed the two new requests, but am still working on fixing the logging tests. I'll continue tomorrow. |
- Reverse names of `log_evaluating_condition_result` and `log_evaluating_condition_final_result` to better reflect their usage - Move condition_count check to caller for `log_evaluating_condition_result` - Move `log_evaluating_targeting_rules` to its proper alphabetical position ♻️ Refactor log_evaluating_condition_final_result Pass a `newline?` param instead of `condition_count` to prepare for a better calculation of `newline?`. Involved moving some increase_/decrease_indent calls around. ✨ Pass newline_before_then? flag Based on the final evaluated condition, determine whether or not there should be a newline before the `THEN` in the logs. This was a bit tricky to properly accumulate and pass around.
Compute and return a `newline_before_then?` flag and pass it to the logger.
|
@adams85 I now have all of the tests passing. I had to add the I believe I've now addressed all of your feedback, so I think this should be good to merge. Let me know if you find anything else. |
|
Thanks for taking the trouble. 🙏 As for me, the PR is complete. |
NOTE: This PR is based on #116 and the base branch has been set accordingly. I'll rebase before merging.
Describe the purpose of your pull request
Features/improvements:
config_v6.json) and update config modelEvaluationDetails.matched_targeting_rule/matched_percentage_optionproperties (rename + set combinations correctly)ConfigCatClient.getKeyAndValueFromSettingsMapTests:
EvaluationDetails.matched_targeting_rule/matched_percentage_optionproperties (rename + set combinations correctly)Related improvements/fixes:
Related issues (only if applicable)
https://trello.com/c/wpNt5Lqu/23-catnip
Requirement checklist (only if applicable)