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

Implement getFeatureVariable and create tests #298

Merged
merged 13 commits into from
Jul 15, 2019
Merged

Conversation

brandonhudavid
Copy link
Contributor

@brandonhudavid brandonhudavid commented Jun 28, 2019

Summary

  • Implement getFeatureVariable method to return value of variable with any type
  • Create new unit tests to ensure functionality of getFeatureVariable

Since Javascript is a dynamically-typed language, it suffices to have a single method return the value of a feature variable rather than have a separate method for each possible feature variable type.

Test plan

  • Create new unit tests to ensure functionality of getFeatureVariable
    • New unit tests similar to tests for getFeatureVariable with specific type
  • Create gherkin integration tests using FSC and javascript-testapp

Issues

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 97.615% when pulling 3926a57 on get_feat_var_1.0 into f1ce881 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 97.615% when pulling 3926a57 on get_feat_var_1.0 into f1ce881 on master.

@coveralls
Copy link

coveralls commented Jun 28, 2019

Coverage Status

Coverage increased (+0.07%) to 97.597% when pulling de5449e on get_feat_var_1.0 into ae941da on master.

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

Almost there! Just a few changes.

packages/optimizely-sdk/lib/optimizely/index.js Outdated Show resolved Hide resolved
packages/optimizely-sdk/lib/optimizely/index.js Outdated Show resolved Hide resolved
@mikeproeng37
Copy link
Contributor

Also, please merge the latest master into your branch and push that with your next commit.

Copy link

@thomaszurkan-optimizely thomaszurkan-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. Mike's comments seem spot on. Nice!

@brandonhudavid brandonhudavid changed the title Implement getFeatureVariable and create unit tests Implement getFeatureVariable and create tests Jul 9, 2019
@mikeproeng37
Copy link
Contributor

Passing build here: https://travis-ci.com/optimizely/fullstack-sdk-compatibility-suite/builds/119044040 using FSC PR: https://github.com/optimizely/fullstack-sdk-compatibility-suite/pull/303

@aliabbasrizvi please unblock this when you get a chance so we can merge this in and then merge in the FSC PR.

Copy link
Contributor

@aliabbasrizvi aliabbasrizvi 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.

@mikeproeng37 mikeproeng37 merged commit f650aaa into master Jul 15, 2019
@brandonhudavid brandonhudavid deleted the get_feat_var_1.0 branch July 15, 2019 18:48
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.

5 participants