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

Feature/migrate async summers from sb #296

Merged
merged 3 commits into from
Apr 16, 2014

Conversation

ianoc
Copy link
Collaborator

@ianoc ianoc commented Apr 7, 2014

We had these summers in summingbird for a while, seems like a better home for them is in Algebird however.

@jcoveney
Copy link
Contributor

jcoveney commented Apr 8, 2014

This does see like a more reasonable home. I guess the one question is whether we want to have this be in util? I know that @johnynek is on record as hating util packages and I tend to agree.

@ianoc
Copy link
Collaborator Author

ianoc commented Apr 8, 2014

-util here refers to having a dependency on twitter-util, not that it is util code

@jcoveney
Copy link
Contributor

jcoveney commented Apr 9, 2014

Lol, that's embarassing. twitter-util is still poorly named, then :P ok will give it one last lookthrough and then merge

def add(t: T) = addAll(Iterator(t))
def addAll(vals: TraversableOnce[T]): Future[M]

def isFlushed: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

not a huge sticking point, but imho this is a misleading name as these queues will be flushed many times. hasCachedElements seems more to point but I don't want to spur a bunch of dumb refactoring

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cache is a bad name here, flush applies to a buffer so kind of covers what this should be closer than that.

jcoveney added a commit that referenced this pull request Apr 16, 2014
@jcoveney jcoveney merged commit 968ac15 into develop Apr 16, 2014
@jcoveney jcoveney deleted the feature/migrateAsyncSummersFromSB branch April 16, 2014 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants