Skip to content

Conversation

aliabbasrizvi
Copy link
Contributor

No description provided.

@aliabbasrizvi aliabbasrizvi requested a review from a team as a code owner August 28, 2019 16:51
@aliabbasrizvi aliabbasrizvi removed their assignment Aug 28, 2019
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.635% when pulling fae297f on ali/bump_version into 2fdb089 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.635% when pulling fae297f on ali/bump_version into 2fdb089 on master.

@coveralls
Copy link

coveralls commented Aug 28, 2019

Coverage Status

Coverage decreased (-0.04%) to 97.595% when pulling e7eba18 on ali/bump_version into 2fdb089 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.

Looks ok, I just have a small suggestion

CHANGELOG.md Outdated
$configManager = new HTTPProjectConfigManager(<<SDK_KEY>>);
$optimizely = new Optimizely(<<DATAFILE>>, null, null, null, false, null, $configManager);
```
* The `fetch` method allows you to refresh the config.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's clarify where the fetch method can be called with a small sample snippet. I can see users getting confused and thinking of calling fetch on the client

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Will clarify.

@aliabbasrizvi aliabbasrizvi removed their assignment Aug 28, 2019
@aliabbasrizvi aliabbasrizvi requested a review from a team August 28, 2019 18:17
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.

Ship it!

@aliabbasrizvi aliabbasrizvi merged commit 043a906 into master Aug 28, 2019
@aliabbasrizvi aliabbasrizvi deleted the ali/bump_version branch August 28, 2019 19:03
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.

3 participants