Paul still working on updating the patch, you can expect a new patch
sometime this week. Sorry for the delay.
On 2011/07/08 00:38:14, rbaxter85 wrote:
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/
