Make 'timestamp' a positional argument

[pelted]

#generate() uses positional arguments for everything but the last timestamp keyword argument. This change is more for consistency.

Mixing positional and keyword arguments is normal. This method already has 4 existing positional arguments in front of this one which forces callers to reason more about how to use this method when the last argument is suddenly a keyword. ¹

Footnotes

1. Another, arguably more expandable, approach would be a **options splat and pass timestamp in a hash. ↩

[directionless]

Idiomatically, I'm thinking about this as having 4 required arguments, and then 1 optional timestamp. Potentially, in the future, there may be more optional arguments. Knowing that, do you want to move timestamp to positional?

[pelted]

Knowing that I would expect the method to look something like this

def self.generate(signing_key, challenge_id, challenge_data, request_data, **options)
   options = { timestamp: Time.now.utc.to_i }.merge(options)
   ...
end

or

def self.generate(signing_key:, challenge_id:, challenge_data:, request_data:, **options)
   options = { timestamp: Time.now.utc.to_i }.merge(options)
   ...
end

but I don't think this 2nd one is Ruby 1.9 compatible.

[directionless]

I'm not sure we need ruby 1.9.

I think timestamp: Time.now gives us a better path to get to **options.

Thinking about it, I'm uncomfortable with optional positional arguments. I'd suggest moving to all named style, or leaving as is.