xtrasmal / TacticianBundle

Symfony2 Bundle for the Tactician library -DEPRECATED / MOVED to
https://github.com/thephpleague/tactician-bundle
5 stars 2 forks source link

Support multiple command buses #19

Closed rtuin closed 9 years ago

rtuin commented 9 years ago

Support for multiple command buses, as discussed in https://github.com/xtrasmal/TacticianBundle/issues/6

Todo

rtuin commented 9 years ago

I think this is a great start. What do you think @xtrasmal @rosstuck @rdohms?

Scrutinizer seems to be unhappy, but I'm not sure why. Config error, maybe? Message is:

Waits for external service to send code coverage to Scrutinizer.
Code coverage data is not yet available.
Scrutinizer was notified that the tests failed.
xtrasmal commented 9 years ago

@rtuin I'll merge and play around with it.

rdohms commented 9 years ago

@xtrasmal may i suggest some OSS practices for dealing with PRs?

You can checkout the pr branch and play with it locally without merging the PR, cause if you merge its is now spread to everyone using the library instantly, regardless of being ready.

Since we have no stable release yet, this means you now broke my local environment with dev-master :P

PRs should only be merged when they are final and have had feedback from code-reviews and builds

Not my project, but this is how i usually manage it, so worth mentioning.

rdohms commented 9 years ago

With that, i think the code it top notch @rtuin i fine with how it was implemented.

xtrasmal commented 9 years ago

@rdohms I will do what is wise next PR. I knew what I was doing it was wreckless, but it looked good enough that otwell it in. Next time I'll discuss it. Please except my apologies.

rtuin commented 9 years ago

Thanks for the merge and the access @xtrasmal!

I agree with you both on how to do PR feedback before merging, but that's for next time then :+1: