Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Datafile parsing + audience evaluation for ODP integration #305

Merged
merged 45 commits into from
Aug 4, 2022
Merged

feat: Datafile parsing + audience evaluation for ODP integration #305

merged 45 commits into from
Aug 4, 2022

Conversation

mikechu-optimizely
Copy link
Contributor

Summary

  • Parse new integrations section of the datafile
  • Evaluate() using UserContext

Test plan

Run full OptimizelySDK.Tests project/suite

Issues

  • OASIS-8392

@mikechu-optimizely
Updated user context
3d20b51
@mikechu-optimizely
Added ref to SDK from DemoApp so solution builds.
0b4d1b9
@mikechu-optimizely
Merge branch 'master' into mike/datafile-parsing-audience-evaluation
7542ca6
@mikechu-optimizely
(Datafile)ProjectConfig & Integration added
b80966d
@mikechu-optimizely
Update Condition interface & implementing classes
bac1f1d
@mikechu-optimizely
Add copyright notice
b8022ae
@mikechu-optimizely
Add AttributeType
15f5b63
@mikechu-optimizely
Implement changes from DoesUserMeetAudienceConditions
7915232
@mikechu-optimizely
Correct accessibility & missing interface item
af97fcc
@mikechu-optimizely
Bring AttributeType into BaseCondition
cdc6c29
@mikechu-optimizely
Add missing reference
4c415b3
@mikechu-optimizely
Fix parsing
5cd1eeb
@mikechu-optimizely
Add test for Integration parsing
d998b57
@mikechu-optimizely
Move reference to Integration
7fedccb
@mikechu-optimizely
Correction to NetStandard 2.0 project reference
a9295d2
@mikechu-optimizely
Add empty Integration test
dd62705
@mikechu-optimizely
Test for empty integration and non "odp" integration
5420bf6
@mikechu-optimizely
Fix missed Copyright update
e29a61a
@mikechu-optimizely mikechu-optimizely requested a review from a team as a code owner July 20, 2022 19:28
@mikechu-optimizely mikechu-optimizely changed the title [OASIS-8392] feat: Datafile parsing + audience evaluation for ODP integration feat: Datafile parsing + audience evaluation for ODP integration Jul 22, 2022
msohailhussain
msohailhussain suggested changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

i just did coding review not functional review.

OptimizelySDK.DemoApp/OptimizelySDK.DemoApp.csproj Show resolved Hide resolved
OptimizelySDK.Net40/OptimizelySDK.Net40.csproj Show resolved Hide resolved
OptimizelySDK.Tests/Utils/TestConversionExtensions.cs Outdated Show resolved Hide resolved
OptimizelySDK.Tests/Utils/TestConversionExtensions.cs Show resolved Hide resolved
OptimizelySDK.Tests/Utils/TestConversionExtensions.cs Show resolved Hide resolved
OptimizelySDK/AudienceConditions/AndCondition.cs Outdated Show resolved Hide resolved
OptimizelySDK/AudienceConditions/BaseCondition.cs Outdated Show resolved Hide resolved
OptimizelySDK/Entity/Integration.cs Outdated Show resolved Hide resolved
OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
jaeopt
jaeopt requested changes Aug 2, 2022
View reviewed changes
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

Most changes look good. We see public getter/setter for qualifiedSegments missing in OptimizelyUserContext. I also see several "deprecation" github warnings. Is this normal?

OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
OptimizelySDK.Tests/AudienceConditionsTests/ConditionEvaluationTest.cs Outdated Show resolved Hide resolved
OptimizelySDK.Tests/ProjectConfigTest.cs Outdated Show resolved Hide resolved
OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
@mikechu-optimizely
Copy link
Contributor Author

Most changes look good. We see public getter/setter for qualifiedSegments missing in OptimizelyUserContext. I also see several "deprecation" github warnings. Is this normal?

@jaeopt The deprecation warnings need a separate chore+PR to resolve in my opinion.

jaeopt
jaeopt requested changes Aug 2, 2022
View reviewed changes
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

Looks good! A small change suggested and a clarification.

OptimizelySDK.Tests/ProjectConfigTest.cs Show resolved Hide resolved
OptimizelySDK/OptimizelyUserContext.cs Outdated Show resolved Hide resolved
jaeopt
jaeopt approved these changes Aug 2, 2022
View reviewed changes
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

@mikechu-optimizely mikechu-optimizely merged commit d369886 into master Aug 4, 2022
@mikechu-optimizely mikechu-optimizely deleted the mike/datafile-parsing-audience-evaluation branch August 4, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaeopt jaeopt jaeopt approved these changes

@msohailhussain msohailhussain Awaiting requested review from msohailhussain

Assignees

@mikechu-optimizely mikechu-optimizely

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mikechu-optimizely @msohailhussain @jaeopt