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

Already on GitHub? Sign in to your account

Integrate LiftNamedComet into Lift #1251

Merged
merged 1 commit into from May 14, 2012

Conversation

Projects
None yet
2 participants
Owner

fmpwizard commented Apr 17, 2012

As discussed here
https://groups.google.com/d/topic/liftweb/-gwez1cOo7A/discussion
I'll be working on adding a simple way to have the same comet class on several browser tabs, but all using names so that only certain cometactors get the messages they need.

@ghost ghost assigned fmpwizard Apr 7, 2012

@fmpwizard fmpwizard closed this in d59a033 Apr 17, 2012

@fmpwizard fmpwizard reopened this Apr 17, 2012

Owner

fmpwizard commented Apr 17, 2012

There are a few places on this pull request where I hope I did things right, so I'll comment on those lines to make it easier for you all to give feedback.

I wasn't sure if there was a better data structure than Vector for this variable.
The operations I'm doing here are:
1- Append
2- To remove dead actors I use .fildetrNot(...)
3- Iterate through the whole Vector to send message to each CometActor

See comments on line 65 explaining how come this compiles on 2.8.x.
But I also wonder if you think this is an ok practice or not.

Same question about data structure.
Actions:
1- Append
2- Lookup at random placed based on key

Something I don't do right now is remove LiftActors (the dispatchers) when the last cometActor is removed from the dispatch. Any suggestions on best way to do this?

Did I get the use of Futures right?

From our user's point, they will use this like this:

NamedCometListener.getOrAddDispatchersFor(name).foreach(
  dispatcher=> dispatcher ! registerCometActor(this, name)
)

I wasn't too sure if just by calling .foreach , there was a new thread being used, or if I had to then use Schedule, as I did here.

Owner

fmpwizard commented Apr 17, 2012

Ok, I'm done asking questions :)
Thanks!

dpp pushed a commit that referenced this pull request May 14, 2012

Merge pull request #1251 from lift/diego_issue_1251
Integrate LiftNamedComet into Lift

@dpp dpp merged commit f023e88 into master May 14, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment