Skip to content

[FSSDK-10619] Refactor project config manager to be injectable#945

Merged
raju-opti merged 46 commits intomasterfrom
raju/pconf
Sep 27, 2024
Merged

[FSSDK-10619] Refactor project config manager to be injectable#945
raju-opti merged 46 commits intomasterfrom
raju/pconf

Conversation

@raju-opti
Copy link
Copy Markdown
Contributor

@raju-opti raju-opti commented Sep 20, 2024 •
edited
Loading

Summary

  • This PR allows the user to create a ProjectConfigManager and pass it directly to createInstance. It exposes two factory methods createStaticProjectConfigManager and createPollingProjectConfigManager for this purpose.

Test plan

  • New tests have been added and relevant old tests has been updated

Issues

  • FSSDK-10619
  • FSSDK-10639
  • FSSDK-10641

@raju-opti raju-opti requested a review from a team as a code owner September 20, 2024 10:09
@raju-opti raju-opti marked this pull request as draft September 20, 2024 10:09
@raju-opti raju-opti changed the title project config manager refactor [DRAFT] project config manager refactor Sep 20, 2024
@raju-opti raju-opti changed the title [DRAFT] project config manager refactor [FSSDK-10619] Refactor project config manager to be injectable Sep 20, 2024
@raju-opti raju-opti marked this pull request as ready for review September 20, 2024 15:46
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 20, 2024 •
edited
Loading

Coverage Status

coverage: 89.201% (-1.1%) from 90.307%
when pulling c483d85 on raju/pconf
into e7cc602 on master.

@mikechu-optimizely
Copy link
Copy Markdown
Contributor

mikechu-optimizely commented Sep 20, 2024

Sorry I didn't get to this today. It's first on my list Mon

mikechu-optimizely
mikechu-optimizely approved these changes Sep 23, 2024
View reviewed changes
Copy link
Copy Markdown
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.

LGTM.

junaed-optimizely
junaed-optimizely approved these changes Sep 25, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@junaed-optimizely junaed-optimizely left a comment

Choose a reason for hiding this comment

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

Great work! So many great additions + modifications.
LGTM!
I just have couple of curious questions.

jaeopt
jaeopt approved these changes Sep 26, 2024
View reviewed changes
Copy link
Copy Markdown
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 with a few comments.
PR is too big and includes completely rewritten codes, so hard to review. I see no clear errors, but expect all these changes covered by unit tests (and later with FSC).

@raju-opti raju-opti merged commit 1d814a2 into master Sep 27, 2024
@raju-opti raju-opti deleted the raju/pconf branch September 27, 2024 15:58
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

@mikechu-optimizely mikechu-optimizely mikechu-optimizely approved these changes

@junaed-optimizely junaed-optimizely junaed-optimizely approved these changes

+1 more reviewer

@raju2592 raju2592 raju2592 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@raju-opti raju-opti

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@raju-opti @coveralls @mikechu-optimizely @raju2592 @jaeopt @junaed-optimizely