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

Composer tweaks #15

Merged
merged 2 commits into from
Aug 20, 2014
Merged

Composer tweaks #15

merged 2 commits into from
Aug 20, 2014

Conversation

jsor
Copy link
Contributor

@jsor jsor commented Aug 20, 2014

  1. Moved autoloading of test classes to autoload-dev. This should be a no-brainer.
  2. Made zendframework/zend-db an optional dependency. Not sure if you really want this. I will write a Doctrine adapter for my current project as it already uses Doctrine (i can contribute it back if you're interested), so i don't need Zend\Db. If you don't like this change, feel free to only cherry-pick the other commit.

@jsor
Copy link
Contributor Author

jsor commented Aug 20, 2014

FYI: first version of my Doctrine adapter: https://github.com/dotsunited/prooph-event-store-doctrine-adapter

codeliner added a commit that referenced this pull request Aug 20, 2014
@codeliner codeliner merged commit bd8803b into prooph:master Aug 20, 2014
@codeliner
Copy link
Member

Define zend-db and zend-serializer as optional requirement is a good idea. Your doctrine dbal adapter is a good alternative for all users that are more familiar with doctrine than with zend-db. If you want to use doctrine for the read side it makes sense to use it also as adapter for the write side. Would be great to have your adapter included in ProophEventStore. I will add more adapters in the future like an adapter for mongoDB.

@jsor
Copy link
Contributor Author

jsor commented Aug 20, 2014

Feel free to pull in my adapter.
On the other side, it's probably not a bad idea to keep the adapters in separate packages. One advantage is, that you can test against multiple versions of the integrated library like i did.
That's not easily possible if all adapters are in the same repository.

@codeliner
Copy link
Member

I will follow your suggestion. I will put the ZF2EventStoreAdapter in a separate package, too. This way I can require zend-db and zend-serializer when it's really needed and your package can require doctrine dbal. I will point to both packages in the EventStore readme so everybody can choose his favorite adapter.

@jsor jsor deleted the composer-tweaks branch August 21, 2014 20:31
@jsor
Copy link
Contributor Author

jsor commented Aug 21, 2014

Btw, if you like i can move dotsunited/prooph-event-store-doctrine-adapter over to the prooph organisation.

@codeliner
Copy link
Member

Would be great to have your adapter under the prooph namespace. When the transfer is done, I will give you write access for the repo.

I will change the repository and aggregate handling of the ES in Version 0.5.0. The ES itself will no longer know anything about aggregates and repositories. From 0.5.0 on it will only know streams and their events. It will be the task of the user to organize the streams (f.e. one Stream per Aggregate, one Stream per AggregateType or one Stream for all Aggregates).
The adpater interface will be affected, too.

@jsor
Copy link
Contributor Author

jsor commented Aug 24, 2014

Sounds great!

Feel free to create a repository under the prooph organisation and push my code to it. I tried to move the repo to prooph, but to do this, one need admin rights at the target org.

I'm at vacation currently (and will be for the 2 weeks) and will delete the repo once i'm back.

Note that there 2 differences to the Zend\Db adapter

  1. I allow to pass a serializer adapter to control the serialization format (https://github.com/dotsunited/prooph-event-store-doctrine-adapter/blob/master/src/DoctrineEventStoreAdapter.php#L258)
  2. occuredOn includes microseconds (see https://github.com/dotsunited/prooph-event-store-doctrine-adapter/blob/master/src/DoctrineEventStoreAdapter.php#L259)

@codeliner
Copy link
Member

I've decided to only fork the repo so that everybody can find it on the prooph overview page. If I would create a new repo and only push your code to it the history get lost. It's your work and a good promotion that the EventStore can be extended very easy.

I will send you a pull request when I changed the adapter interface.

@jsor
Copy link
Contributor Author

jsor commented Aug 30, 2014

To keep the history, just create a new repo under prooph, clone my repo, and change the remote to the prooph repo and push.

I think the fork could confuse people becaue they will be unsure what to use. They will prefer the prooph repo since it looks more official but the repo will be mostly not in sync with original.

@codeliner
Copy link
Member

ok, thank you for the tip. I've moved the repo now. Btw. you can find a first draft of the 0.5.0 Version of the ES in current development branch of the EventStore.

@jsor
Copy link
Contributor Author

jsor commented Aug 31, 2014

Cool, i will look at it when i'm back from vacation.

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