Skip to content

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.

@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

6 participants