-
Notifications
You must be signed in to change notification settings - Fork 130
Conversation
array($this, 'onCollected'), | ||
Profiler::PRIORITY_TOOLBAR | ||
); | ||
$this->listeners[] = $events->getSharedManager()->attach('profiler', ProfilerEvent::EVENT_COLLECTED, array($this, 'onCollected'), Profiler::PRIORITY_TOOLBAR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use please short array syntax?
Can we also add to scope of this update for ZF3 following improvements:
|
matrix: | ||
allow_failures: | ||
- php: 7.0 | ||
- ./vendor/bin/phpunit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one empty line at the and of the file plz
@webimpress some feedbacks incorporated. I think phpcs checks and some documentation should be in separate PR :) |
} | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove white characters please
@webimpress updated. |
@webimpress implemented. |
@samsonasik good work! 😄 Thanks! Can we update please I'd like to have also trailing commas in arrays (once you changed it to short syntax). What do you think? BTW. Why you think |
The original PR is to get it support for zf3. Original reasons: I have no chance to add phpcs check right now ( need to play with my sons ) :) Warm regards, Abdul Malik Ikhsan Pada 25 Jun 2016, pukul 13.59, webimpress [email protected] menulis:
|
@webimpress my children finally sleeping, I've finally have a chance to add php-cs check and checks for lowest, locked, latest in .travis.yml ;) |
@webimpress travis now green with php-cs check and lowest, locked, and latest check dependencies included \m/ |
@samsonasik very nice 😄 thanks! I think "locked" is not working properly because we don't have |
@webimpress done ;) |
@webimpress add composer.lock make travis failure, suggestion ? or could you make a patch to my branch ? |
@webimpress nvm, travis has been fixed now and green ;) 💃 💃 💃 |
@samsonasik I'm not sure about this The same library ( Please notice also nice I've also checked In some library I saw also BTW. Do we still need PHP 5.5 support in the library? ZF3 is gonna support PHP 5.6 and PHP 7.0. |
It will be very nice to add this library also to new Zend Skeleton Application (using |
I think phpcs more enhancement should be separate PR, it's already too far :) For php 5.5, yes, we need support as we will still uses in zend-mvc 2. This PR used to support both zf-mvc 2 and zf-mvc 3. Warm regards, Abdul Malik Ikhsan Pada 26 Jun 2016, pukul 07.28, webimpress [email protected] menulis:
|
@@ -22,10 +21,10 @@ | |||
* @author Mark Garrett <[email protected]> | |||
* @since 0.0.3 | |||
*/ | |||
class EventLoggingListenerAggregate implements SharedListenerAggregateInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible BC break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it tested with EventManager v2 and v3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svycka Essentially, that interface goes away with v3, so to make code forwards compatible, you cannot directly implement the interface. Since this particular class implements a method for attaching listeners to the shared manager, and that method is what is called inside the Module
class, it continues to work perfectly.
If I do a new minor or major release of this, we can bump to 5.6 support. 😄 Thanks a TON for the work on this @samsonasik , and for the constant, constructive feedback, @webimpress ! |
Oh, goodness. Evidently we did a release candidate in October, but never a stable release following. I'm going to tag that as 1.0.0, and then this will become 1.1.0. |
I just discovered one fairly big issue: zend-version is deprecated, and we are no longer updating it for any but LTS releases. I'll see if I can remove that from the toolbar. |
cherry-pick #210 with incorporated feedbacks.