[webkit-reviews] review granted: [Bug 5906] webkit.org should explain how to detect the latest nightly : [Attachment 12284] js library

[bugzilla-request-daemon at macosforge.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 5906: webkit.org should explain how to detect the latest nightly
http://bugs.webkit.org/show_bug.cgi?id=5906

Attachment 12284: js library
http://bugs.webkit.org/attachment.cgi?id=12284&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
r=me -- a few small thoughts about refinement

>    return RegExp("AppleWebKit/").test(navigator.userAgent);
>    var webKitFields = RegExp("(.*AppleWebKit/)([^
]*)").exec(navigator.userAgent);

I think you should require a space before AppleWebKit in both of these regular
expressions. Also, I suggest using a + rather than a * in the version number
part of the expression.

>	 return undefined;

Generally I'd suggest returning null instead of undefined. I think undefined
should be reserved for the meaning "this isn't defined at all".

> WebKit.versionIsAtLeast = function versionIsAtLeast(minimumString)

Why do the functions themselves have names as well as the name of the property
of the WebKit object? Is this helpful?

>    // Defaults to ""
>    minimumString = String(minimumString);

I don't understand the value of this line of code. Are we trying to make this
function work if people call with things that aren't strings?

Are you sure that WebKit is the best name for this object? I think it works
well for the WebKit.versionXXX functions, but not as well for the
WebKit.isWebKit.