optimizely / csharp-sdk

.NET based C# SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/csharp-sdk
Apache License 2.0
19 stars 20 forks source link

feat: Integrated ODPManager with UserContext and Optimizely client #323

Closed mikechu-optimizely closed 1 year ago

mikechu-optimizely commented 1 year ago

Summary

This PR integrates ODP functionality with UserContext and Optimizely client to make it available for users. There are two ways to integrate ODPManager.

😬 Sorry about the numerous auto-format changes polluting the core enhancements. Need to satisfy the linter.

Test plan

  1. Manually tested thoroughly
  2. Will add unit tests after the first review passes.

Issues

FSSDK-8476

msohailhussain commented 1 year ago

@mikechu-optimizely I had a discussion with Jae internally and there are few suggestion you need to make, internally DM you.

mikechu-optimizely commented 1 year ago

@msohailhussain & @jaeopt, I'm ready for another round of reviews.

I am still worried about how big this PR has become. Sorry.

mikechu-optimizely commented 1 year ago

Ready for another set of reviews ... or a LGTM4N Approval 😉 @msohailhussain @jaeopt @mnoman09

mikechu-optimizely commented 1 year ago

@mnoman09 I see where the FSC is failing. Is that what #325 is for?

mikechu-optimizely commented 1 year ago

Still holding for @msohailhussain 's approval (maybe if he's time).

@mnoman09 I'll hold for your merge and Approval too.

mikechu-optimizely commented 1 year ago

@msohailhussain @mnoman09 It looks like some FSC tests are erroring for this branch. Shall I start investigating the test app?

mnoman09 commented 1 year ago

@msohailhussain @mnoman09 It looks like some FSC tests are erroring for this branch. Shall I start investigating the test app?

I can look at the testapp. Most of these tests are failing because we haven't merged the testapp PR: 79. Once we merge that PR FSC should pass.

mikechu-optimizely commented 1 year ago

Great. Thank you.

mikechu-optimizely commented 1 year ago

Is there something I can be helping with for getting FSC/test-app scenarios corrected?

@mnoman09 @msohailhussain

msohailhussain commented 1 year ago

Is there something I can be helping with for getting FSC/test-app scenarios corrected?

@mnoman09 @msohailhussain

Is there something I can be helping with for getting FSC/test-app scenarios corrected?

@mnoman09 @msohailhussain

Is there something I can be helping with for getting FSC/test-app scenarios corrected?

@mnoman09 @msohailhussain

I will do by tomorrow.

mikechu-optimizely commented 1 year ago

I'm starting to understand FSC and the C# testapp. Working to figure out failing tests...

mikechu-optimizely commented 1 year ago

Thanks for the approval. FSC and the test app are my last blocker

mikechu-optimizely commented 1 year ago

OMG @msohailhussain and @mnoman09 You guys are the best.

Valuable lesson for me is to keep PR's much smaller.

I'll start working on the full-repo linting.

Thanks again, you guys.