Skip to content
This repository has been archived by the owner on Jun 20, 2021. It is now read-only.

Naming: device_token instead of device #13

Closed
dennisreimann opened this issue Apr 20, 2013 · 5 comments
Closed

Naming: device_token instead of device #13

dennisreimann opened this issue Apr 20, 2013 · 5 comments

Comments

@dennisreimann
Copy link

Hello Mattt,

what do you think about changing the name of the device variable/parameter in the notification class to device_token? I think this would make it a bit clearer, especially in the context of using houston along with rack-push-notification. I just tripped over this, passing a device instance into the notification initializer, instead of giving it the device.token (which works, but will fail silently later on)

It's not a very big deal but imho this would make the notifications interface clearer. I know this would be a breaking change, but I think it's worth considering, because even without the rack-push-notification context this could make it more intuitive.

Cheers!

@mattt
Copy link
Contributor

mattt commented Apr 24, 2013

I think you're absolutely right. Everywhere in the code except for the instance variable, it's referred to as a token, so the one-off is confusing. 926590c renames device to token while maintaining backwards compatibility. Thanks for your recommendation!

@mattt mattt closed this as completed Apr 24, 2013
@dennisreimann
Copy link
Author

Awesome, thanks! 🍰

@dennisreimann
Copy link
Author

One more thing: Can you push a new version with this change, or are you planing to integrate some more stuff into 0.2.2?

@mattt
Copy link
Contributor

mattt commented Apr 30, 2013

Just pushed 0.2.2. Enjoy!

@dennisreimann
Copy link
Author

\o/ :cake:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants