Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTN5 #98

Merged
merged 2 commits into from
Jan 14, 2016
Merged

RTN5 #98

merged 2 commits into from
Jan 14, 2016

Conversation

ricardopereira
Copy link
Contributor

No description provided.

for _ in 1...max {
disposable.append(ARTRealtime(options: options))

let channel = disposable.last?.channel(channelName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using last?, why not simply declare the client to make the code more readable such as:

let client = ARTRealtime(options: options)
disposable.append(client)
let channel = client.channel(channelName)

@mattheworiordan
Copy link
Member

Nice, encouraging to see that work! 👍

@ricardopereira ricardopereira self-assigned this Jan 4, 2016
@ricardopereira ricardopereira force-pushed the RTN5 branch 3 times, most recently from df1895b to 31e8d08 Compare January 5, 2016 23:08
@ricardopereira ricardopereira removed their assignment Jan 6, 2016
 - Misplaced
 - Sometimes it was not reaching the number of messages. e.g. channel
was not ready to receive the message
@tcard
Copy link
Contributor

tcard commented Jan 14, 2016

LGTM

@tcard
Copy link
Contributor

tcard commented Jan 14, 2016

@mattheworiordan I think this was rebased, so I'm not sure if anything changed since your comment. Please merge if LGTY.

@mattheworiordan
Copy link
Member

LGTM

mattheworiordan added a commit that referenced this pull request Jan 14, 2016
@mattheworiordan mattheworiordan merged commit cf023a5 into master Jan 14, 2016
@ricardopereira ricardopereira deleted the RTN5 branch January 15, 2016 11:09
maratal pushed a commit that referenced this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants