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

GraphStage rewrite #22

Closed
wants to merge 2 commits into from
Closed

GraphStage rewrite #22

wants to merge 2 commits into from

Conversation

gafiatulin
Copy link
Contributor

Fixes #21

@coveralls
Copy link

Coverage Status

Coverage decreased (-29.4%) to 50.0% when pulling 2ad93d2 on gafiatulin:graph-stage into e209c8f on 2gis:master.

@savulchik savulchik requested a review from LMnet August 18, 2017 02:28
@savulchik
Copy link
Member

@gafiatulin thanks for contributing! I will give you feedback soon)

@coveralls
Copy link

Coverage Status

Coverage decreased (-30.7%) to 48.649% when pulling 57752b3 on gafiatulin:graph-stage into e209c8f on 2gis:master.

@savulchik savulchik self-assigned this Aug 19, 2017
@savulchik
Copy link
Member

@gafiatulin I'd like to cherry-pick library updates and cross building into master in order to release 0.3.0 version of the library having ActorPublisher implementation. After that I'd ask you to rebase GraphStage implementation on updated master.

BTW, don't you mind to split the latest commit into another PR? I'd like to discuss the approach of using typeclasses like Unmarshaller[A] for abstracting over return type.

@savulchik
Copy link
Member

@gafiatulin I released 0.3.0 version, now you may rebase your branch on master

@coveralls
Copy link

coveralls commented Aug 19, 2017

Coverage Status

Coverage decreased (-29.4%) to 50.0% when pulling 171355a on gafiatulin:graph-stage into cab79eb on 2gis:master.

@gafiatulin
Copy link
Contributor Author

@savulchik I rebased on current master.
Also dropped polymorphic source commit. Going to open new issue concerning that in a minute.

@gafiatulin
Copy link
Contributor Author

#23 is what I was looking for in the first place with that polymorphic source commit. I did it that way because it requires changes in interactions with underlying jeromq (ZMsg.recvMsg(s) vs s.recv).

Not really sure about unmarshalling being done in source. Sources shouldn't be concerned with deserializing of incoming data into domain entities. Especially not with my initial approach.

@gafiatulin gafiatulin closed this by deleting the head repository Jan 26, 2024
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.

3 participants