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

Raise an error on RealtimeUpdates::validate_update if @secret is not set #318

Merged
merged 1 commit into from Sep 28, 2013

Conversation

Projects
None yet
4 participants
@theosp
Contributor

theosp commented Aug 4, 2013

RealtimeUpdates::validate_update requires @secret to be set in the RealtimeUpdates
instance.

But the RealtimeUpdates can be initialized without it (with @app_access_token instead),
in which case validate_update will break.

Raise an error on RealtimeUpdates::validate_update if @secret is not set
RealtimeUpdates::validate_update requires @secret to be set in the RealtimeUpdates
instance.

But the RealtimeUpdates can be initialized without it (with @app_access_token instead),
in which case validate_update will break.
@pboling

This comment has been minimized.

Show comment
Hide comment

pboling commented Aug 5, 2013

👍

@@ -125,6 +125,10 @@ def self.meet_challenge(params, verify_token = nil, &verification_block)
# end
# end
def validate_update(body, headers)
if @secret == nil

This comment has been minimized.

@romanbsd

romanbsd Aug 5, 2013

Contributor

@secret.nil? is more idiomatic ruby

@romanbsd

romanbsd Aug 5, 2013

Contributor

@secret.nil? is more idiomatic ruby

This comment has been minimized.

@arsduo

arsduo Sep 28, 2013

Owner

I actually ended up going with an unless :)

@arsduo

arsduo Sep 28, 2013

Owner

I actually ended up going with an unless :)

@arsduo

This comment has been minimized.

Show comment
Hide comment
@arsduo

arsduo Sep 28, 2013

Owner

Thanks! Sorry this took me a bit; I'm definitely a fan of better errors. I've added a test and merged this in.

Owner

arsduo commented Sep 28, 2013

Thanks! Sorry this took me a bit; I'm definitely a fan of better errors. I've added a test and merged this in.

@arsduo arsduo merged commit 81c68e7 into arsduo:master Sep 28, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment