Drew, I have just set the timeout to the values that we want. Do you want a new webrev?
Thanks, John On Aug 15, 2011, at 11:57 AM, Drew Fisher wrote: > John, > > My only comment on the code stems from the fact that you're calling > int(GLOBAL) a bunch. Wouldn't it make more sense to either: > > a) set the global, with the cast > > IMAGES_TIMEOUT = int(4.0 * 1000) > > or ... > > b) just set the value that you want? > > IMAGES_TIMEOUT = 4000 > > It doesn't look like those timeouts are configurable in any way, so why do > all the unnecessary math/casts? > > Otherwise this looks fine to me. > > -Drew > > > > > > On 8/15/11 12:34 PM, John Fischer wrote: >> All, >> >> I still need one more review. >> >> Thanks, >> >> John >> >> On Aug 13, 2011, at 7:26 AM, John Fischer wrote: >> >>> Dermot, >>> >>> I had forgotten that when we spoke that I was going to use a unique timer >>> name. Sorry about that. I have updated the below webrev accordingly for >>> both issues: >>> >>> https://cr.opensolaris.org/action/browse/caiman/johnfisc/7076130-2 >>> https://cr.opensolaris.org/action/browse/caiman/johnfisc/7076130-diff >>> >>> Thanks, >>> >>> John >>> >>> >>> On 08/13/11 01:12 AM, Dermot McCluskey wrote: >>>> Hi John, >>>> >>>> Are you re-using the image timer interval value for the >>>> new "finished" timer? It might be clearer to have a separate >>>> global var for the new timer interval. >>>> >>>> Also, would line 342 be better placed after line 383? ie >>>> don't assume success until you have verified there were >>>> no errors? >>>> >>>> Otherwise, looks good. >>>> >>>> - Dermot >>>> >>>> >>>> >>>> On 8/13/2011 12:23 AM, John Fischer wrote: >>>>> All, >>>>> >>>>> Can I get a review for CR 7076130: >>>>> >>>>> http://monaco.us.oracle.com/detail.jsf?cr=7076130 >>>>> 7076130 [A11Y]GUI installor hangs when starting accessibility >>>>> installation. >>>>> >>>>> The webrev is located at: >>>>> >>>>> https://cr.opensolaris.org/action/browse/caiman/johnfisc/7076130/ >>>>> >>>>> This is a high priority fix because the install appears to hang when >>>>> indeed it has >>>>> completed. I'll send email to Dave asking for integration approval. >>>>> >>>>> The failure was due to the GUI being updated from a thread other then the >>>>> main thread. >>>>> The use of threading.Timer() needs to be replaced to glib.timeout_add() >>>>> instead as well. >>>>> The solution is to establish the timeout and from within the callback >>>>> check to see if >>>>> the installation has completed. >>>>> >>>>> I have installed a system with and without using Accessibility technology. >>>>> All installs now work properly. >>>>> >>>>> Thanks, >>>>> >>>>> John >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> [email protected] >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> _______________________________________________ >> caiman-discuss mailing list >> [email protected] >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
