Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
Stefan Goethals
@zipkid
Good morning.
Maybe you can take a look at what was messing with https://github.com/zipkid/slack-rtmapi/commits/feature/web_api
Stefan Goethals
@zipkid
@mackwic is there a specific reason why you restricted the RTM to only open, message, and error events?
Thomas Wickham
@mackwic
Hello !
@rosylilly this is nice !
@zipkid no specific reasons. Can recall me what are the other messages ?
@zipkid could you push your branch on mackwic/slack-rtmapi (via git push mackwic feature/web-api for instance) and then open a PR so that the review would be easy (and I can contribute, as we are both contributors on mackwic/slack-rtmapi and not on zipkid/slack-rtmapi)
Stefan Goethals
@zipkid
@mackwic the RTM API events list https://api.slack.com/events
@mackwic i'll push the branch now.
Voila.
Thomas Wickham
@mackwic
oh yeah, it was postpone, the time I found a simple and elegant way to dispatch them. I didn't think that people will start use slack-rtmapi the week i published it. ;)
thanks for the PR
Stefan Goethals
@zipkid
Hey, it was already over 2 weeks when i started :-P
Thomas Wickham
@mackwic
:)
zipkid @zipkid is interested in 'code style' critique too...
Thomas Wickham
@mackwic
ok
Thomas Wickham
@mackwic
@zipkid aaaaaand, review done ! some issues are stylistic, some are organizational, the only two thing that bother me are
1) the Thread in inner_loop, which should be moved in another class (a wrapper like SlackRTM::ThreadedClient or SlackRTM::Client::Threaded
2) the little (but multiple) separation of concern infraction. It's not too bad at our stage of the project, but still, I want to keep it as clean as possible as I plan for a lot of extensions...
in order to finish the 1), I think we will need to rework slightly the SlackRTM::Client to be more composable (don't need much more, but I saw a couple of blocking points)
Stefan Goethals
@zipkid
1) i was not sure how to handle this thread thing but it seemed to work as i did, i'm open to improvements:-)
2) i'm using this as a plugin in another project too :-)
Stefan Goethals
@zipkid
Good morning.
I guess we need to figure out what to do about the RT in RTM in slack-rtmapi.
With the blocking .readpartial this example https://gist.github.com/zipkid/22be5ae8edb003f3c5c2 does not run well..
What can be done about that?
Stefan Goethals
@zipkid
I'm experimenting with read_nonblock ...
Stefan Goethals
@zipkid
Ok... i tried another way, comitting now.
mackwic/slack-rtmapi@b1aba5f
Stefan Goethals
@zipkid
I just discovered the 'open, message, and error ' events are not from Slack but from websocket-driver ... :-)
Thomas Wickham
@mackwic
yep
sorry, I have a lot of work right now. Can't be responsive
what's the issue with readpartial ?
I think it's good to expose the two ways of reading, the blocking one and the nonblocking one
sorry again for not being here when you are active :/
Stefan Goethals
@zipkid
What is the advantage of having a blocking read?
Thomas Wickham
@mackwic
1) simplicity
2) efficience: no hoverhead, most efficient way to get the bytes from the OS perspective
3) interoperablity: very easy to plug into various systems that doesn't provide async IO
both are needed
Stefan Goethals
@zipkid
Did you look at my solution...?
I think it answers to these 3 and does provide 'Real Time' instead of 'delayed until we get network data'
Thomas Wickham
@mackwic
didn't had time, sorry for that :/
I'll check soon, promised
Thomas Wickham
@mackwic
back in the race. Time to kick some PRs ! :)
Stefan Goethals
@zipkid
Hi
Stefan Goethals
@zipkid
?
Stefan Goethals
@zipkid
Good morning.
Stefan Goethals
@zipkid
Hello, hello, is this still alive?
Thomas Wickham
@mackwic
Hey hello !
@zipkid I am totally sorry, I am in vacations now, and my business work is pretty intensive right now.
Again. sorry for not being there.
You should work on a branch or on your repo, and as soon as business will calm we will merge that quietly.
Thomas Wickham
@mackwic
This message was deleted