Skip to content

[FSSDK-8452] feat: Added ODP Manager Implementation #797

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
Merged
merged 10 commits into from
Feb 24, 2023
Merged

[FSSDK-8452] feat: Added ODP Manager Implementation #797

merged 10 commits into from
Feb 24, 2023

Conversation

@opti-jnguyen
Copy link
Contributor

Summary

Added ODPManager Implementation which does the following.

  1. Initializes and provides access to ODPEventManager and ODPSegmentManager.
  2. Provides updated ODPConfig settings to event manager and segment manager.
  3. Stops Event Manager when closed.
  4. Uses VUID for browser flavor of javascript SDK.

Test plan

  1. Manually tested thoroughly
  2. Added unit tests.

Issues

FSSDK-8452

@opti-jnguyen opti-jnguyen requested a review from a team as a code owner February 11, 2023 02:57
mikechu-optimizely
mikechu-optimizely previously requested changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@mikechu-optimizely mikechu-optimizely left a comment

Choose a reason for hiding this comment

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

I have a few small questions listed.

jaeopt
jaeopt requested changes Feb 13, 2023
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 few changes suggested.

jaeopt
jaeopt requested changes Feb 17, 2023
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.

All changes look great. Some small changes suggested.

jaeopt
jaeopt requested changes Feb 21, 2023
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.

A couple of more suggestions.

@opti-jnguyen opti-jnguyen requested a review from jaeopt February 22, 2023 22:06
jaeopt
jaeopt approved these changes Feb 22, 2023
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

@coveralls
Copy link

Coverage Status

Coverage: 92.959%. Remained the same when pulling 2540239 on jnguyen/odp-manager into ce4224d on master.

@Tamara-Barum Tamara-Barum changed the title feat: Added ODP Manager Implementation [FSSDK-8452] feat: Added ODP Manager Implementation Feb 24, 2023
@opti-jnguyen
Copy link
Contributor Author

Merging without passing FSC for velocity, will address at a later point.

@opti-jnguyen opti-jnguyen merged commit 0116d75 into master Feb 24, 2023
@opti-jnguyen opti-jnguyen deleted the jnguyen/odp-manager branch February 24, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikechu-optimizely mikechu-optimizely mikechu-optimizely left review comments

@jaeopt jaeopt jaeopt approved these changes

Assignees

@opti-jnguyen opti-jnguyen

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@opti-jnguyen @coveralls @jaeopt @mikechu-optimizely