[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