[wp-trac] [WordPress Trac] #33587: Notification Functions Should be Replaced by Hooks
WordPress Trac
noreply at wordpress.org
Fri Sep 4 20:09:35 UTC 2015
#33587: Notification Functions Should be Replaced by Hooks
-------------------------+------------------------------
Reporter: dshanske | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Awaiting Review
Component: Mail | Version:
Severity: normal | Resolution:
Keywords: has-patch | Focuses:
-------------------------+------------------------------
Comment (by boonebgorges):
dshanske - Thanks for the idea, and for the patch! I agree that it'd be
nice to handle these via hooks, if only so that they can be easily
removed.
A general question about strategy. You've torn the guts from the existing
functions and replaced those guts with `do_action()` calls. IMO, it makes
more sense to replace the direct calls to `wp_notify_postauthor()`, etc
with `do_action()` calls. In some cases, you may need very small wrappers.
See [33587.2.diff] for a suggested technique. This has the advantage of
making the notifications unhookable, but greatly reduces the amount of
churn (and new function names) that your patch requires. And if you don't
like the idea of a wrapper like `wp_send_comment_notifications()`, we
could introduce new actions inside of `wp_new_comment()` - maybe
`'comment_post_not_approved'` and `'comment_post_author_notify'`.
Is there a reason why this sort of thing wouldn't work?
Some more general notes about the patch:
* As mentioned in Slack, it makes testing much easier if the patch is
generated from the WordPress root.
* In the absence of extenuating circumstances, new function names should
start with `wp_`.
* Tabs not spaces for indentation.
* When using `add_action()` or `add_filter()`, no need to pass `1` as the
fourth param (`$accepted_args`). 1 is assumed as default.
* All calls to `add_action()` and `add_filter()` should go in `default-
filters.php` (or `ms-default-filters.php`, or `wp-admin/includes/admin-
filters.php`, or `wp-admin/includes/ms-admin-filters.php`, as
appropriate).
--
Ticket URL: <https://core.trac.wordpress.org/ticket/33587#comment:8>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list