fschuindt/firebase_id_token

Consider rename `request_anyway` to `request!`

ababich opened this issue · 4 comments

ababich commented

request! looks more ruby-way :) Please, consider the naming

fschuindt commented

That's true. At the beginning when I was starting this gem, the actual names were .request and .request!. After some time I decided to change .request! to .request_anyway, 'cause I thought the following:

Trailing bangs in Ruby implies that the data/sate was changed, like:

@user = User.find(id)
@user.refresh_token!

In this case, the example method .refresh_token! will change the state of @user.

I am not sure, but I think that's the main convention reason, what do you think?

I think it's better to consider the name change after some discussion about it.

Thank you!
Best regards,

ababich commented

Sure, but these methods are class methods and they change the state of the system
I see analogue more like ActiveRecord::Base#save and save! which both do pretty the same thing but with different level of reporting on failures

For me it just sounds not very straightforward what _anyway does from the point of generally accepted ruby "dictionary". Of course, this is compact gem with a few methods and it id not any issue. At least now

fschuindt commented

So, I have set it as a TODO for the next release. 😄
I will close it after released.

Thank you for your help.

  • 👍1
fschuindt commented

It's implemented now.
request_anyway was kept as deprecated for compatibility reasons.