svalaskevicius / eventflow

cqrs+es in scala
45 stars 4 forks source link

add projection example for multiple aggregates #16

Closed svalaskevicius closed 8 years ago

svalaskevicius commented 8 years ago

@Fristi what do you think? can you spot anything that could be improved easily?

svalaskevicius commented 8 years ago

Hi @Fristi, any feedback on this version?

currently the projection can be replayed from the start (events ordered by their creation) regardless of where in the process was it added.

there are still some items to refactor, esp direct usage of the inMemoryDb in projections

Fristi commented 8 years ago

Added some comments here and there.

Overall I need to take a closer look what all constructs are doing conceptually.

svalaskevicius commented 8 years ago

ok, looks a bit better now I hope :)

@Fristi can you spot anything that has to be fixed before merging?

Fristi commented 8 years ago

Overall I think it's nice to get projections out there! There are some things we could improve here :) Sometimes I find it not easy to digest some of you code. How could we improve on that? I think with a few modifications we can clarify it a lot, what do you think?

I can help you out with refactoring some parts! For now it would be good to merge this :+1:

svalaskevicius commented 8 years ago

completely agree! :) in fact, I thought I am already doing most of those items :D apart from short names and no docs.

happy to update the missed parts - in the end, I will have to read this as well even after I forget how it was implemented :)

will have a skim over again and see what have I missed, thanks!

svalaskevicius commented 8 years ago

@Fristi are these places that you meant - https://github.com/svalaskevicius/eventflow/commit/8a34878c930644b01866e42c03d894c008300d95 ? happy to improve more if you see something obvious I missed again :)

will merge - but we can always improve this in other PRs (and I hope we will :) )

and thanks for the review!