Thanks for making a first pass Paul. Is this the tool your referring
to?
http://code.google.com/p/closure-linter/source/browse/trunk/closure_linter/gjslint.py?r=2
On 2011/07/07 16:03:37, Paul Lindner wrote:
First pass -- didn't look at tests or examples closely.
Please run the js code through gjslint, that will catch most of the
issues I
pointed out.
http://codereview.appspot.com/4641058/diff/1/content/samplecontainer/examples/commoncontainer/cconviews.js
File content/samplecontainer/examples/commoncontainer/cconviews.js
(right):
http://codereview.appspot.com/4641058/diff/1/content/samplecontainer/examples/commoncontainer/cconviews.js#newcode29
content/samplecontainer/examples/commoncontainer/cconviews.js:29: *
@param
{string}
{string=} for optional params here and throughout
http://codereview.appspot.com/4641058/diff/1/content/samplecontainer/examples/commoncontainer/cconviews.js#newcode140
content/samplecontainer/examples/commoncontainer/cconviews.js:140:
$("#dialog_"
+ dialog_counter).dialog({
do we really need jquery here?
http://codereview.appspot.com/4641058/diff/1/content/samplecontainer/examples/commoncontainer/cconviews.js#newcode298
content/samplecontainer/examples/commoncontainer/cconviews.js:298:
"ui-widget
ui-widget-content ui-helper-clearfix
ui-corner-all").find(".portlet-header")
I think all of these css classes here and throughout should have a
unique prefix
so that they don't possibly clash. Bonus points if you can make the
prefix
configurable..
http://codereview.appspot.com/4641058/diff/1/features/pom.xml
File features/pom.xml (right):
http://codereview.appspot.com/4641058/diff/1/features/pom.xml#newcode9
features/pom.xml:9: * with the License. You may obtain a copy of the
License at
chunk mismatch here.. weird..
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements-container.js
File
features/src/main/javascript/features/views/viewenhancements-container.js
(right):
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements-container.js#newcode67
features/src/main/javascript/features/views/viewenhancements-container.js:67:
*
@param {Object}
Object.<string, string|Object>=
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements-container.js#newcode85
features/src/main/javascript/features/views/viewenhancements-container.js:85:
if
(orig_site != undefined && orig_site.getActiveGadgetHolder() !=
undefined){
prefer !== form where possible.
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements-container.js#newcode150
features/src/main/javascript/features/views/viewenhancements-container.js:150:
*
url: URL to a web page to open in a URL site in the
container. (Note
this should not
bad jsdoc format. Try running gjslint to get some help.
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements-container.js#newcode155
features/src/main/javascript/features/views/viewenhancements-container.js:155:
*
@param {string}
string= here and throughout for opt_ params
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements.js
File features/src/main/javascript/features/views/viewenhancements.js
(right):
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements.js#newcode22
features/src/main/javascript/features/views/viewenhancements.js:22:
These are probably not needed if you have the deps set up correctly.
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements.js#newcode66
features/src/main/javascript/features/views/viewenhancements.js:66: *
@param
{string}
{string=}
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements.js#newcode89
features/src/main/javascript/features/views/viewenhancements.js:89: *
@param
retValue:
Type? {Object}?
http://codereview.appspot.com/4641058/diff/1/features/src/main/javascript/features/views/viewenhancements.js#newcode100
features/src/main/javascript/features/views/viewenhancements.js:100: *
@param
resultCallback:
{function} here I believe.
http://codereview.appspot.com/4641058/
