@diosmosis opened this Pull Request on September 22nd 2017 Member

Added a couple new events that are triggered when changes are made to dynamic files on a Piwik server. The events include:

  • Core.configFileChanged: triggered when an INI config file is changed
  • CoreAdminHome.customLogoChanged: triggered when a custom logo/favicon file is updated
  • CustomPiwikJs.piwikJsChanged: triggered when tracker JS is regenerated & saved
@tsteur commented on September 23rd 2017 Owner

Added 2 small comments but looks good otherwise 👍

@diosmosis commented on September 23rd 2017 Member

@tsteur Made the requested changes.

@tsteur commented on September 23rd 2017 Owner

PR looks good to me. I would probably renamed one event to size_$size instead of size=$size or just named it ... customLogoChanged.$size but totally fine with equal size as well. @diosmosis just wondering do we actually need this second event with the size directly in the event name? Totally fine to leave it in there, just wondering if it is actually needed.

FYI: We will need to see what kind of documentation it generates on https://developer.piwik.org/api-reference/events#controllermoduleaction for that event (once merged) as I don't remember whether we need some kind of extra logic for that on the developer website or not.

@diosmosis commented on September 25th 2017 Member

I would probably renamed one event to size_$size instead of size=$size or just named it ... customLogoChanged.$size but totally fine with equal size as well.

I was going by the text in your comment. Is there a documented convention?

@diosmosis just wondering do we actually need this second event with the size directly in the event name?

Not needed for what I'm doing, just figured I'd be thorough.

@tsteur commented on September 25th 2017 Owner

There is no convention for this. If it is not needed, I would maybe remove the event though.

@diosmosis commented on September 25th 2017 Member

Removed the size=$size events.

This Pull Request was closed on September 25th 2017
Powered by GitHub Issue Mirror