[buddypress-trac] [BuddyPress] #5148: Make notifications a separate component

buddypress-trac noreply at wordpress.org
Sat Sep 7 01:13:33 UTC 2013


#5148: Make notifications a separate component
----------------------------------+------------------------------
 Reporter:  johnjamesjacoby       |       Owner:  johnjamesjacoby
     Type:  defect (bug)          |      Status:  new
 Priority:  normal                |   Milestone:  2.0
Component:  Notifications         |     Version:
 Severity:  normal                |  Resolution:
 Keywords:  needs-patch needs-ui  |
----------------------------------+------------------------------

Comment (by boonebgorges):

 Patch looks really good, r-a-y. I'm a big fan of moving the miscellaneous
 methods into central `delete()` and `get()` methods. Also, the fact that
 you fixed a couple of these basic but major bugs is crazy to me - I guess
 that means that few people are hacking on Notifications at the moment.
 (Heck, I've hardly ever hacked on it.)

 Couple small comments:
 - Since we're essentially gutting a couple of user facing functions and
 replacing them with the new all-purpose `get` and `delete` methods, we
 should have unit tests for the procedural wrappers. Then we can rest
 assured that the refactor didn't break anything.
 - `get_where_sql()` looks good to me, but let's mark it `protected
 static`. It's probably a bad idea to encourage developers to use BP's
 utility methods for building partial SQL statements, as it binds our hands
 if we ever want to make internal syntax modifications.

 But yeah, these look like very solid improvements for 1.9.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5148#comment:4>
BuddyPress <http://buddypress.org/>
BuddyPress


More information about the buddypress-trac mailing list