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.

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

Create virtual methods on user context #280

Conversation

daniel-pons
Copy link
Contributor

@daniel-pons daniel-pons commented Sep 6, 2021

The following versions of Optimizely C# SDK would use the methods defined on the class OptimizelyUserContext to perform some processes, E.G: Decide, TrackEvent, DecideAll, etc...

Then, it is required on the vast majority of unit testing frameworks, for mocking this object to be able to override such members.

This can be accomplished, at least on C#, usually in two ways:

  1. Declaring an interface (with the methods to be overriden) and having the class implementing it.
  2. Declaring the members as virtual, would allow also allow the testing framework to override them.

This PR uses the second approach, it makes the methods on OptimizelyUserContext virtual.

Also fixes a problem with development environments with a culture different from English.

@daniel-pons daniel-pons requested a review from a team as a code owner September 6, 2021 10:08
@daniel-pons daniel-pons changed the title Daniel pons/create virtual methods on user context Create virtual methods on user context Sep 6, 2021
@msohailhussain
Copy link
Contributor

can you please update it with master.

daniel-pons and others added 3 commits September 8, 2021 13:55
Pull changes from source repository
…irtual

Make OptimizelyUserContext members virtual, making them overridable by testing frameworks
…re to english, as for systems on others cultures, some tests were failing.
@daniel-pons daniel-pons force-pushed the daniel-pons/create-virtual-methods-on-user-context branch from c770664 to 3fbdfcf Compare September 8, 2021 12:01
@daniel-pons
Copy link
Contributor Author

can you please update it with master.

My bad, done!

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.

lgtm

@msohailhussain msohailhussain merged commit c1233ce into optimizely:master Sep 13, 2021
@daniel-pons
Copy link
Contributor Author

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants