[RFC|Merged] General Hook Architecture of phpBB3.1

[brunoais]

@ nickvergessen
I like it.

[Oleg]

The doc proposal looks great.

Anyone familiar with whatever documentation system we do or can use to automatically extract it?

[nickvergessen]

This should be in a wiki page of hook guidelines for future reference

As mentioned, it will be put into the wiki once the post is confirmed.

Also, instead of core.group_remove_user, would core.group.remove_user make more sense?

we decided to use only one dot for the main prefix. everything else should use underscores

[nickvergessen]

Should events be wrapped with if ($phpbb_dispatcher != null) to avoid problems in all the functions, if the dispatcher is not set up (f.e. in tests or external usage)

[MichaelC]

Should events be wrapped with if ($phpbb_dispatcher != null) to avoid problems in all the functions, if the dispatcher is not set up (f.e. in tests or external usage)

I'd say no. The dispatcher should always be setup (this DIC will make this easier).

[naderman]

You need to do that if you write code that allows $phpbb_dispatcher to be null. So basically it's a case by case decision, ideally if the dispatcher is passed as an argument it's defined by whether or not you add "= null" as a default as null will not be allowed otherwise. In cases where you still use it as a global variable, i guess it's really just a matter of whether you see a reasonable use case of calling the function without a dispatcher.

[imkingdavid]

Why not instead of null check to see if it's a proper object...

Code: Select all

if ($phpbb_dispatcher instanceof phpbb_event_dispatcher)
{
    // Event code here
} 

[igorw]

IMO we should create a null dispatcher (or a mock) and pass that one instead.

[MichaelC]

Shouldn't this be marked as merged and a separate RFC created for template events?

[Oleg]

If someone creates a new RFC for template events this RFC can be merged.