[WIP] Implemented model observers #50
Conversation
tests/Functional/EventsTest.php
Outdated
| /** @dataProvider getEventsData */ | ||
| public function testEvents($setup, $execute, array $expectedEvents) | ||
| { | ||
| ob_start(); |
There was a problem hiding this comment.
Nope, just back from a holiday and my brain isn't thinking 100% straight yet ;) Used a better technique now.
|
Ready to merge imo. Will wait for a day or 2 before merging and releasing 0.4.0. If anyone has some time to review, please do. |
| { | ||
| public function boot() | ||
| { | ||
| User::observer('app.user_observer'); |
There was a problem hiding this comment.
if the observers are registered as services, wouldn't it be possible to avoid that step to the user?
There was a problem hiding this comment.
At first, I thought: Awesome idea!
However it turns out it's quite a bit harder than you think. So I created a new issue for it: #52
There was a problem hiding this comment.
Fair, I'll try to have a look at the end of the week, I don't recall when what is booted and when to be booted
| private $symfonyContainer; | ||
| private $observers; | ||
|
|
||
| public function __construct(ContainerInterface $symfonyContainer, array $observers) |
There was a problem hiding this comment.
What do you mean? I can't typehint array any more specific, right?
| /** | ||
| * @author Wouter J <wouter@wouterj.nl> | ||
| */ | ||
| class Events_ClassTest extends EventsTest |
There was a problem hiding this comment.
why the partial snake case here?
There was a problem hiding this comment.
Yeah, a bit weird personal convention :) Will remove the _
| /** | ||
| * @author Wouter J <wouter@wouterj.nl> | ||
| */ | ||
| class Events_ServiceTest extends EventsTest |
I would agree with tagging everything |
|
Fair for the final, in the end it's just a personal preference.
For the array type I meant being more specific with phpdoc blocks.
…On Mon, 6 Mar 2017 at 18:46, Wouter J ***@***.***> wrote:
I left a few comments. Besides that WDYT of making those classes final? I
think it's better to have everything as final by default.
I would agree with tagging everything @Final, but I'm not in favor of
marking them final in PHP. I don't think I should enforce people not to
extend them, but I can tell them I don't help/support them if they do :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#50 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE76gZWiOLHkVPhVSLAKTc67fEp69nz2ks5rjFRogaJpZM4MTfsB>
.
|
|
I don't think the PHPdoc will explain more than the method signature. I'll add all finals in a follow up PR after merging this one. Thanks for your review! |
|
You're welcome. Do you have any ETA for a stable release or a beta/RC? Not urgent but I would prefer this lib to be somewhat stable when releasing the stable version of https://github.com/theofidry/AliceDataFixtures. |
|
@theofidry I'm hoping to release 0.4 stable this week. After that, I only have "nice to have" features left in the issue list afaics. So that means 0.5 might as well become 1.0. I'm not sure when I'll release 0.5/1.0, but it shouldn't take that long as there aren't much things left (other than Laravel 5.4 support). The 0.x versions are already quite stable though. |
/fixes #6, #42
Todo