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 18:56:43 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?

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. 


- 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