Can you elaborate on how you have responded to Max and my previous review 
comments on this change?

I find Max's technique, where he quotes the review comments and explains 
point-by-point how he addressed (or why he is not addressing) each one as part 
of the updated change request to be really helpful.  It makes it so the 
reviewer does not have to play "hunt the wumpus" to figure out if his comments 
have been addressed or not.

Thanks!

On 2010-08-24, at 15:31, Henry Minsky wrote:

> Max, is it possible for you test this change with Webtop, to see if it still 
> operates? The query arg handling should be back compatible
> but I'm guessing there might be some configuration I didn't cover in testing
> 
> (If you're too busy to fire up Webtop, I can try to do it on my machine.. )
> 
> 
> 
> Change hqm-20100824-ocl by [email protected] on 2010-08-24 11:29:17 EDT
>    in /Users/hqm/openlaszlo/trunk-clean
>    for http://svn.openlaszlo.org/openlaszlo/trunk
> 
> Summary: add unified 'lzoptions' support
> 
> New Features:
> 
> Bugs Fixed: LPP-3479
> 
> Technical Reviewer: ptw
> QA Reviewer: max
> Doc Reviewer: (pending)
> 
> Documentation:
> 
> Release Notes:
> 
> Overview:
> 
> + Added "flexversion" compiler option to select between flash 10.0 and 10.1
> 
> + Added support for passing compiler options in unified "lzoptions" string
> 
> + modified "lzc" command line interpreter to accept 'flexversion' arg
> 
> Details:
> 
> 
> WEB-INF/lps/lfc/kernel/swf/LzBrowserKernel.lzs
> WEB-INF/lps/lfc/kernel/swf9/LzBrowserKernel.lzs
>       added getLzOption method, which looks at the 
> lz.embed.applications.options field
> 
> WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs
>       Added routine to parse lzoptions string
> 
> WEB-INF/lps/config/lps.properties
>       add default value for flex version = 10.0
> 
> WEB-INF/lps/server/src/org/openlaszlo/utils/LZHttpUtils.java
>       for back compatibility treat "runtime" as a synonym for "lzr" in 
> lzoptions
> 
> WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9External.java
>       pass "flex_version" option to flex compiler (targets 10.0 vs 10.1)
> 
> WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java
>       Add "flexversion" constant
> 
> WEB-INF/lps/server/src/org/openlaszlo/server/LPS.java
>       add method to return default value for flex version
> 
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderCompile.java
>       use accessor LZHttpUtils.getLzOption to get arg values instead of 
> request.getParameter
> 
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderAPP_CONSOLE.java
>       formats the lzoptions values into the XML data format used by the 
> dev-console.lzx app
> 
> WEB-INF/lps/server/src/org/openlaszlo/compiler/CompilationEnvironment.java
> WEB-INF/lps/server/src/org/openlaszlo/compiler/Compiler.java
> WEB-INF/lps/server/src/org/openlaszlo/compiler/Main.java
> WEB-INF/lps/server/src/org/openlaszlo/compiler/SWF9Writer.java
>       add support for 'flex version' arg
> 
> 
> lps/includes/source/embednew.js
> lps/includes/source/build.xml
>       add parser for lzoptions string
>       store parsed options in "options" slot on application, similar to 
> initargs
> 
> lps/admin/dev-console.lzx
>       generate a URL string for the app being loaded that encodes args into  
> lzoptions format
> 
> + I did not modify "lzc" command line processor to accept the new lzoptions
> format yet. It still requires the old style discrete flags. 
> 
> + solo deploy wizards still use old style compiler options in query
> string when pulling data from the server, I will create a JIRA task to
> update them
> 
> Tests:
> smokecheck in swf10
> test/lfc/data in swf10
> 
> TLF bidi tests
> test/tlf/text-test.lzx?lzr=swf10
> test/tlf/focus-input.lzx?lzr=swf10
> 
> fetching test/hello.lzx using lzoptions
> http://127.0.0.1:8080/trunk-clean/test/hello.lzx?lzoptions=lzr(swf10),flexversion(10.1)
> http://127.0.0.1:8080/trunk3/test/hello.lzx?lzoptions=runtime(swf8),debug
> http://127.0.0.1:8080/trunk3/test/hello.lzx?lzoptions=runtime(dhtml),debug,lzbacktrace
> 
>  verify that dev-console displays correct button and checkbox settings for 
> the selected options 
>   (e.g., runtime and debug/backtrace flags, remote console debug)
> 
> confirm by inspection of as files in compiler apache temp build
> directory 'lzswf9', that flex compiler is getting passed the arg 
> "-target-player=10.1":
> /openlaszlo/lib/apache-tomcat-5.5.29/temp/lzswf9/Users/hqm/openlaszlo/trunk-clean/test/build/hello/build.sh
> 
> 
> test/data/jdata.lzx: 
>  modified to test proxied flag, it displays the value of canvas proxied flag
>  test with urls
>  test/data/jdata.lzx?lzoptions=proxied(false)  // new lzoptions arg
>  test/data/jdata.lzx?lzoptions=proxied(true)
>  test/data/jdata.lzx?lzproxied=false    // old style query arg
>  test/data/jdata.lzx?lzproxied=true
> 
> 
> Files:
> 
> M       test/data/jdata.lzx
> M       WEB-INF/lps/lfc/kernel/swf/LzBrowserKernel.lzs
> M       WEB-INF/lps/lfc/kernel/dhtml/LzBrowserKernel.lzs
> M       WEB-INF/lps/lfc/kernel/swf9/LzBrowserKernel.lzs
> M       WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs
> M       WEB-INF/lps/lfc/services/LzBrowser.lzs
> M       WEB-INF/lps/lfc/debugger/platform/swf/LzDebug.as
> M       WEB-INF/lps/lfc/debugger/platform/swf9/LzFlashRemote.as
> M       WEB-INF/lps/config/lps.properties
> M       WEB-INF/lps/server/src/org/openlaszlo/cm/CompilationManager.java
> M       WEB-INF/lps/server/src/org/openlaszlo/utils/LZHttpUtils.java
> M       WEB-INF/lps/server/src/org/openlaszlo/cache/DataCache.java
> M       WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9External.java
> M       WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java
> M       WEB-INF/lps/server/src/org/openlaszlo/server/LPS.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderCompile.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderAPP_CONSOLE.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderOBJECT.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/TemplateResponder.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderLFC.java
> M       
> WEB-INF/lps/server/src/org/openlaszlo/compiler/CompilationEnvironment.java
> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/Compiler.java
> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/Main.java
> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/SWF9Writer.java
> M       lps/includes/source/embednew.js
> M       lps/admin/dev-console.lzx.swf
> M       lps/admin/dev-console.lzx
> M       lps/admin/lps/includes/lfc/LFCdhtml.js
> M       lps/admin/solo-deploy.jsp
> M       lps/admin/dev-console.lzx.js
> M       lps/admin/solo-dhtml-deploy.jsp
> 
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/hqm-20100824-ocl.tar


Reply via email to