-
Notifications
You must be signed in to change notification settings - Fork 36
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
Delivery rules correction for proper rollout key #351
Conversation
…p, which was invalid.
@msohailhussain @jaeopt Python seems to pass FSC here without any errors which is interesting... V2 Config custom build - https://app.travis-ci.com/github/optimizely/fullstack-sdk-compatibility-suite/builds/233550803 |
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.
One comment to clarify test cases. Other than that, LGTM
…er Config Document.
…nabled in variations_map
To trigger continuous integration, I sometimes try to add or change a comment in .gitignore. I like to change the first line from '#' to '##' and also from '##' to '#'. |
If you log into travis, you can actually just go to the build and trigger it there rather than pushing up new commits. I usually just do that, unless I have some functional item I am checking like None verse False. |
I'm not sure if that updates the statuses on GitHub. I think this is useful for failing builds on our public repos--we have had some intermittent issues in python. |
…e delivery_rules to use feature_id to get variation variables.
Good call, I think you may be right on that. Rerunning from CI directly does not update the public build status. |
@@ -133,7 +133,7 @@ def __init__(self, project_config): | |||
optly_audience = OptimizelyAudience( |
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.
If you want, you can take a ref of typed_audiences using []
.
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.
Actually going to make it simpler and just use project_config.typed_audiences reference in the loop rather than assign something new. Was doing things a bit differently previously and never cleaned up. Thanks for spotting.
…was unnecessary. Remove str cast as also not needed.
All tests passing! |
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.
Looks ok to me.
Summary
Test plan
Issues