aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David McLaughlin" <da...@dmclaughlin.com>
Subject Re: Review Request 21523: Make JS compliant with JSHint rules
Date Fri, 16 May 2014 19:01:23 GMT


> On May 16, 2014, 5:05 p.m., Mark Chu-Carroll wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 17
> > <https://reviews.apache.org/r/21523/diff/1/?file=582876#file582876line17>
> >
> >     Why?
> 
> David McLaughlin wrote:
>     Good question.
>     
>     JavaScript in the browser doesn't have modules or linking. It just has one big global
scope. In practice linking is done by relying on global variables. JSHint forces you to be
explicit about what globals your project (see the predef option in build.gradle) each file
relies on, so that it can then detect use of undeclared variables elsewhere. 
>     
>     I added all of the core libraries we use to our build.gradle. So these globals are
just leaky values we've exposed ourselves that really should be injected via DI (but that's
a much bigger change). 
>     
>     Adding :false after the global name tells JSHint to throw an error if someone tries
to redeclare a global in some local scope, or to use a local variable with the same name.

Sorry, I didn't finish editing that first paragraph. That final sentence should be:

JSHint forces you to be explicit about what globals you use, which can be set for the entire
project (see the predef option in build.gradle) or on a per-file basis with a comment. By
knowing all expected globals, JSHint can then detect use of undeclared variables.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21523/#review43218
-----------------------------------------------------------


On May 16, 2014, 1:30 a.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21523/
> -----------------------------------------------------------
> 
> (Updated May 16, 2014, 1:30 a.m.)
> 
> 
> Review request for Aurora, Suman Karumuri and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-438
>     https://issues.apache.org/jira/browse/AURORA-438
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Make JS compliant with JSHint rules
> 
> 
> Diffs
> -----
> 
>   build.gradle ac54257a0342e1f1a162189191d5f7b27e1651c9 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 1a617129da4747584df1b49c34ffcaa4b505d3dc

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 1fa1cea08d15109c96a2cd72c97fc1cc7b8fd0a7

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 890cb445aaed29d60d1728c2ad10074912993ad8

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js a2aea4d10e4401efebe37eab10d5ce72ceb5c6ca

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js f984956c90e1db38573a56af3427f2f948f13692

> 
> Diff: https://reviews.apache.org/r/21523/diff/
> 
> 
> Testing
> -------
> 
> $ ./gradlew build
> :about
> :bootstrapThrift UP-TO-DATE
> :checkPython
> :generateSources UP-TO-DATE
> :compileGeneratedJava UP-TO-DATE
> :processGeneratedResources UP-TO-DATE
> :generatedClasses UP-TO-DATE
> :compileJava UP-TO-DATE
> :processResources
> :classes
> :jar
> :assemble
> :jsHint UP-TO-DATE
> :checkstyleMain
> :compileTestJava
> :processTestResources
> :testClasses
> :checkstyleTest
> :findbugsGenerated
> :findbugsMain
> :findbugsTest
> :licenseGenerated
> :licenseTest UP-TO-DATE
> :license UP-TO-DATE
> :test
> :jacocoTestReport
> Coverage report generated: file:////Users/dmclaughlin/t/incubator-aurora/dist/reports/jacoco/test/html/index.html
> :check
> :build
> 
> BUILD SUCCESSFUL
> 
> Total time: 3 mins 19.979 secs
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message