User "Brion VIBBER" posted a comment on MediaWiki.r89082. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/89082#c17398 Commit summary:
Adding or adjusting front-end function and variable documentation per our conventions. Also did some trivial clean up, JSHint fixes and performance optimizations while at it. (per http://www.mediawiki.org/wiki/JavaScript_performance ) * Missing semicolons. * [mediawiki.js Line 540] Unreachable 'registry' after 'throw'. * [mediawiki.log.js] Checked for window.console but used console. * Added callback option to mw.util.toggleToc (forwarded to jQuery.fn.slideUp/Down) * Made mw.log argument more descriptive (logmsg instead of string) * Using deepEqual(, true) instead of ok() when asserting true. (ok() is like casting a boolean (or a plain if() statement) to verify that something is not falsy) Added missing test suites for: * mw.util.toggleToc (follow-up r88732, Dummy check was introduced in r87898) /me has been infected by Reedy and will be doing more of this tonight. Comment: Sure for cases where you really are checking the returned data type yes that makes sense -- and distinguishing a false from failed-to-return-value is valuable. It's the numerous cases of simple boolean comparisons with only two possible values getting deep-compared against 'true' that seem very strange to me. :) "OK" vs "not OK" is exactly what we probably want for simple boolean conditions like this; there are no other possible values and seeing that we got 'false' instead of 'not OK' adds no information; it should still be showing the actual text comment that explains the purpose of the comparison, right? _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
