xwp / site-performance-tracker

Allows you to detect and track site performance metrics
GNU General Public License v2.0
96 stars 15 forks source link

Use hooks for adding markers #4

Closed kasparsd closed 5 years ago

kasparsd commented 5 years ago

Using hooks such as do_action( 'site_performance_mark', 'position' ); instead of conditional methods calls would allow disabling the plugin on production and avoid any PHP errors if the plugin is not enabled.

delawski commented 5 years ago

@kasparsd I actually thought about such approach but read somewhere that using hooks inside templates is not recommended. Plugins like AMP for WP are using a function along with a function check like this:

if ( function_exists( 'is_amp_endpoint' ) && is_amp_endpoint() ) {
  ...
}

I don't know why they decided to go this way.

Still, using a hook as you proposed seems much more reasonable and clean to me. Do you see any potential drawbacks of the hooks-based approach?

kasparsd commented 5 years ago

I don't know why they decided to go this way.

Same here. It looks like an anti-pattern to me.

Do you see any potential drawbacks of the hooks-based approach?

I can't think of any.

kasparsd commented 5 years ago

We could implement a nice namespace approach to the hook names, similar to how ACF does it with acf/field_group/admin_head. We could have xwp/performance_tracker/mark or something.

delawski commented 5 years ago

Sounds good to me!