[wp-trac] [WordPress Trac] #23216: Create "WP Heartbeat" API

WordPress Trac noreply at wordpress.org
Wed Jul 17 00:08:05 UTC 2013


#23216: Create "WP Heartbeat" API
--------------------------------------+-----------------------
 Reporter:  azaozz                    |       Owner:
     Type:  task (blessed)            |      Status:  reopened
 Priority:  normal                    |   Milestone:  3.6
Component:  Administration            |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  autosave-redo needs-docs  |
--------------------------------------+-----------------------

Comment (by azaozz):

 I would have really appreciated another pair of eyes on this couple of
 months ago, when heartbeat was in development. Restructuring/rewriting
 parts of it in RC seems too late.

 Currently the API is in "experimental" stage. The usefulness of the
 initial idea was severely reduced by concerns it will overwhelm shared
 hosting accounts because it connects every 15 seconds while a user is
 logged in. It is structured as a (pseudo) class so it can have private and
 public vars and methods, however that class is intended to be initialized
 only once and is not intended to be extendable.

 In that terms the _Cashe and _Settings objects in 23216-heartbeat-
 cleanup.diff​ ate pointless. If the concerns that connecting every 15
 seconds is too much to handle for an average host are proven to be true,
 heartbeat would probably be reduced to couple of helper functions. If
 these concerns are proven wrong, heartbeat can be made into a "real" JS
 class (or couple of classes) that can be extended, and maybe support
 several instances running at the same time, etc.

 Regarding coding standards:
 - The use of curly brackets on `if` is optional in the WordPress coding
 standard. This is mostly a readability thing as the minimizer adds and
 removes brackets where needed, so the production (minimized) code is
 unchanged.
 - A single space is required after `if`, `for`, `while`, etc.
 - The `else` and `else if` should be on the same line as the preceding
 curly bracket.
 - You mention this couple of times but I don't see any missing/added
 semicolons in 23216-heartbeat-cleanup.diff :)

 Using `switch () { }` instead of a long list of `if () { } else if () { }
 else if () { }`, seems more readable. But this is a personal preference.

 Wrapping a DOM node in jQuery is very fast (it bypasses all the css
 selectors logic, etc.). Not sure caching of `$(document)` and `$(window)`
 makes any significant difference but doesn't hurt. I suppose `$(document)`
 is as readable as `$document` for most people, `_Cache.$document` might
 not be.

 Having a lot of inline comments is nice, explaining too much may feel a
 bit strange though:
 {{{
 // set the value of hasFocus to false
  _Settings.hasFocus = false;
 }}}

 Like the function names changes, not sure all functions in the private
 scope have to start with an underscore. Don't see this anywhere else in
 WordPress. Also not sure it needs all the new private functions. Maybe
 they make heartbeat a little bit slower after the patch.

--
Ticket URL: <http://core.trac.wordpress.org/ticket/23216#comment:96>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list