shareup / phoenix-apple

3 stars 0 forks source link

Timeouts are too confusing #11

Closed atdrendel closed 4 years ago

atdrendel commented 4 years ago

The word "timeout" means many different things in the project. For example, in Channel, the word "timeout" seems to refer to a backoff before trying to reconnect as opposed to a something like a network timeout. Let's be clearer with naming.

Additionally, there may be too many different kinds of timeouts in the project. ChannelPush.timeout, Socket.timeout, Error.joinTimeout Socket.heartbeatTimeout. They might all be needed, but it would be good to reduce their number, if possible.

myobie commented 4 years ago

The name of Channel.timeout is the same as what the javascript uses and should be used currently for the same purpose. The problem could be that in javascript one uses setTimeout and so they might mean "delay" or even just "span of time" instead of timeout. I do think we should link to the javascript if we do rename something so it's clear what it's other side is.

atdrendel commented 4 years ago

That makes sense. The other alternative would be to add /// documentation to our types to describe their purpose more accurately. I'll try that first in order to keep the naming as similar as possible across our implementation and the canonical JavaScript implementation.

atdrendel commented 4 years ago

This has been resolved by https://github.com/shareup/phoenix-apple/pull/2. Specifically, I added comments describing the meanings of the cases of Channel.JoinTimer, which helped clarify the meaning of the join and rejoin timeouts.