[OpenLayers-Trac] Re: [OpenLayers] #3008: Build: Integrate Closure Compiler on OpenLayers

[OpenLayers]

#3008: Build: Integrate Closure Compiler on OpenLayers
---------------------+------------------------------------------------------
 Reporter:  jorix    |       Owner:        
     Type:  feature  |      Status:  new   
 Priority:  minor    |   Milestone:  Future
Component:  general  |     Version:  2.10  
 Keywords:           |       State:        
---------------------+------------------------------------------------------

Comment(by jorix):

 Replying to [comment:7 crschmidt]:

 The goal of this ticket is to use the full power of the compiler to check
 the code.

 I think in version 2.11 when I wrote this ticket. I'll try to answer your
 comments referring to the features of the ticket.

 Main Features:
  * ['''A'''] Do not build if there are any undefined variable
  * ['''B'''] Avoid massive warnings (as in VERBOSE)
  * ['''C'''] Show line number and source file name of the errors and
 warnings

 Additional Features:
  * ['''D'''] Detect some problems in the documentation (see issues
 identified in #2989)
  * ['''E'''] Allow the use of Closure Inspector (['''C'''] is also
 necessary)
  * ['''F'''] Allow flexibility when compiling: to turn off ['''A'''], or
 activate ['''D''']
  * ['''G'''] Allow in the cfg files to use files over the lib folder

 From my point of view the objective ['''A'''] is very important.
 !OpenLayers has some bugs of this type. The latest came in the mobile code
 sprint (btw, great job!, awesome!) in r11535. Eg, this is the message that
 this ticket generated to compile the current trunk code:

 {{{
 ..\lib\OpenLayers\Layer\Grid.js:705: ERROR - variable tlViewPort is
 undefined
         tlViewPort = tlLayer.add(offsetX, offsetY);
         ^
 1 error(s), 26 warning(s)

 Abnormal termination due to compilation errors.
 ERROR: Closure Compilation failed! See compilation errors.
 }}}

 I think this is a good argument ;-)

  (To use the compiler as another way to test the code)

 .

 > jorix: Before looking at this patch (in response to another closure
 compiler patch) I went ahead and added support for closure compiler to the
 existing build tools.

 I had issues with the new closure.py: blank spaces in the path, and lock
 the temporary files. In the end it worked only using no temporary files
 (MS Vista)

 > This functionality is much simpler than what you've built, but it seems
 like you were really building something much more complex than I think
 needs to be included in OpenLayers; essentially, you were replicating the
 entire command line parsing of Closure in OpenLayers.

 Not exactly, the first two options are specific for !OpenLayes ['''F'''].

 > I think that it is reasonable to debug OpenLayers in closure by simply
 building an uncompressed build (python build.py -c none) then running
 closure on the resulting file with the options you want.

 But do not get the names of source files and line numbers of errors
 ['''C''']

 > I expect if we make a shift to ADVANCED_OPTIMIZATIONS we may need to
 enable more complex parsing and optimization, but for now I've simply
 enabled the default closure options (SIMPLE), and things seem to work
 okay.

 This ticket '''is designed to use SIMPLE_OPTIMIZATIONS'''

 > I'm interested in what else this patch is trying to achieve. I see the
 following:
 >  * build/closure-compiler/OpenLayers/externs/errors-pending-to-fix-
 undefined-variables.js looks pretty simple: just various bugfixes that I
 should go review and fix. No problem.
 >  * It looks like build/closure-compiler/OpenLayers/externs/Externs.js is
 simply designed around preventing warnings, something I'm not super-
 concerned about at the moment

 Only to allow ['''A''']

 >  * lib/OpenLayers/Events.js looks to be the same

 Also to can use ['''A''']

 >  * tools/mergejs.py Seems to be based around letting closure find the
 files individually, instead of in one big chunk? is that right? ...

 Yes, yes. To can alow ['''C''']

 > ...And I assume that comes in part from the desire to support the jsDoc
 stuff in closure? ...

 This is not the reason, is still ['''C''']

 > ... the jsDoc stuff in closure? Can you explain this part of the patch
 more?
 >  * tools/NaturalDocs2JsDoc.py looks interesting for future work, but not
 neccesary right now

 It's long to explain ['''D''']. The idea is to send to the compiler
 !NaturalDocs comments translated into jsDoc but keeping the source file
 names and line numbers. If you seems interesting, I have more thing to say
 about this feature...

 >  * build/closure-compiler/closure-
 compiler/contrib/externs/google_maps_api_v2.js I'm assuming is to prevent
 more errors?

 Yes, to can use ['''C''']

 > I assume that these externs and so on would be more neccesary if we were
 going to approach full closure ADVANCED support, ...

 No, only to allow ['''A''']

 > ... but with the simple support, things seem to be working without, ...

 Yes, yes, yes: I've experimented with the ADVANCED_OPTIMIZATIONS and I
 think it is very difficult to use in the future with !OpenLayers, I have
 more thing to say about this...

 > ... so I'm going to hold off on moving forward on reviewing the work in
 this ticket until we're ready to approach closure more thoroughly (or we
 find a bug in the current closure behavior).
 >
 > Thanks for the pointer!


 I am at your disposal.

-- 
Ticket URL: <http://trac.openlayers.org/ticket/3008#comment:8>
OpenLayers <http://openlayers.org/>
A free AJAX map viewer