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

App key #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

App key #21

wants to merge 3 commits into from

Conversation

tomkiefhaber
Copy link

@tomkiefhaber tomkiefhaber commented May 23, 2017

This should prevent users from having to pass the app_key into every
request even when they've set it in the config. It also allows them to
pass in an app_key if they've not gone through the config step, or they
have multiple app_keys and wish to be explicit with which one they use.

Example:

Yotpo.configure do |config|
  config.app_key = ENV['yotpo_key']
  config.secret = ENV['yotpo_secret']
end

# falls back to the Yotpo.app_key from configuration
Yotpo.get_product_bottom_line(product_id: "fake-product")

# uses provided app_key
Yotpo.get_product_bottom_line(app_key: 'totallydifferentappkey', product_id: "fake-product")

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3208ed9 on tomkiefhaber:app-key into f70e0f7 on YotpoLtd:master.

This should prevent users from having to pass the app_key into every
request even when they've set it in the config.  It also allows them to
pass in an app_key if they've not gone through the config step, or they
have multiple app_keys.

Example:

```ruby
Yotpo.configure do |config|
  config.app_key = ENV['yotpo_key']
  config.secret = ENV['yotpo_secret']
end

Yotpo.get_product_bottom_line(product_id: "fake-product")

Yotpo.get_product_bottom_line(app_key: 'totallydifferentappkey', product_id: "fake-product")

```
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 19291cd on tomkiefhaber:app-key into 1551e3f on YotpoLtd:master.

@tomkiefhaber
Copy link
Author

If this is not desirable to anybody, I'll close it. Also happy to make any changes recommended.

@JackCA
Copy link

JackCA commented Feb 15, 2018

This seems like a good idea -- it's really strange that there are steps to configure the app key and secret, and then we must continue to pass it in for each call...

@@ -1,3 +1,3 @@
module Yotpo
VERSION = '1.0.1'
VERSION = '1.0.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put the version change in a separate commit / PR.

Choose a reason for hiding this comment

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

Why did Yotpo not use this PR? Coming to this gem now, and it seems crazy to provide a known key on each and every call.

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