API changes

[mattheworiordan]

Don't action yet, but take a look at ably/ably-js#33 where we are proposing a final set of API changes before we roll out stable versions.

[rjeczalik]

ACK

[rjeczalik]

Comments from #13:

• rename ably.Params (ably.Config? ably.Options) [https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28314800]
• merge RestPresence with RestChannel [https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28313825, https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28437606]; rationale:

However I would opt for merging Presence with Channel. Separate struct made sense when we had ResourceReader and we needed Presence to implement it. Even implementation of the wiring suggests it should be one struct:

func newRestChannel(name string, client *RestClient) *RestChannel {
    c := &RestChannel{
        Name:   name,
        client: client,
    }
    c.Presence = &RestPresence{
        client:  client,
        channel: c,
    }
    return c
}

I'd just extend RestChannel with two methods: Presence(PaginateParams) *PaginatedResource and PresenceHistory(PaginateParams) *PaginatedResource.

• rename Params.AppSecret -> Params.APISecret, Params.AppId -> Params.APIID (according to the rule that acronyms should be capitalised, however we can discuss it further) [https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28314879]
• refactor Params.Tls -> Params.NoTLS (TLS is on by default) [https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28314899]
• rename Params.ApiKey -> Params.Key [https://github.com/Merge config, rest and realtime into one ably package #13#discussion_r28314915]

[kouno]

However I would opt for merging Presence with Channel. Separate struct made sense when we had ResourceReader and we needed Presence to implement it. Even implementation of the wiring suggests it should be one struct:

I see where you are going with that but I think it's a step too far to merge these two. Doing something like ably.NewRestChannel().PresenceHistory() would definitely work and not require much changes, but the only value you gain would be to keep everything in the channel.go file (and yes, saving a few lines of code ;)).

On the other hand, the libraries are all defining a Presence object and if you do this, the documentation would deviate again, which causes us to have to handle an edge case in our textile files (https://github.com/ably/docs/blob/d31ebe5667ec547f87798827d438f168a579834f/content/rest/presence.textile). Also, I think it's nice to have this RestPresence struct still available on the rest part as it is the counterpart of the realtime one where you will definitely not be able to merge it into the realtime channel go file. See https://github.com/ably/ably-ruby/blob/6a5f9607f18519ae0f58927d11a187371cda4091/lib/ably/realtime/presence.rb.

We could discuss this if you feel like this is a good addition (we are very open to ideas), but I think we are too far in the development of our product to implement this now :(.

[mattheworiordan]

Unfortunately I have to agree with @kouno. One of Ably's primary objectives was to provide a consistent (as possible) API across all languages. I don't see any technical reason why we can't mostly achieve that with Go so I'd prefer to keep it as similar as possible please.

[rjeczalik]

@kouno @mattheworiordan Clear to me, no need for further discussion, I'm not strongly opinionated about merging Presence with Channel.

[rjeczalik]

Sent #17 to fix the API. Please give me a note when you're done discussing changes in ably/ably-js#33, and I'll address those in #12.

[mattheworiordan]

I am closing this issue as I will summarise all API changes in one issue