incubator-flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Om <bigosma...@gmail.com>
Subject Re: Erik de Bruin's patch for FLEX-33149 and FLEX-33150
Date Mon, 06 Aug 2012 07:41:24 GMT
Erik, you rock!  Thanks for fixing everything.  The only change I made was
to load the .md5 file from *https*://
www.apache.org/dist/incubator/flex/4.8.0-incubating/binaries/apache-flex-sdk-4.8.0-incubating-bin.zip.md5instead
of over http.

This patch has been committed into svn now.

I have tested it in a couple of locations, once the
http://api.hostip.info/country.php service returned: XX, i.e. it could not
resolve the country.  In this case the app download failed and aborted.  I
had to go to hostip.info to add my details.  The next time I ran it worked
fine.

The hostip.info service does not look foolproof.  As an extra backup,
should we try to parse the the closer.cgi html like Ravindra Bharathi
mentioned in this thread:
http://markmail.org/message/2zte5zseu4wpidx6 ?


> 2.  You have added a few log mesasges.  Can you please convert them into
> > locale strings for the en_US locale at least?  This will make it easier
> for
> > other locale contributors to provide corresponding translations.
>
> Done. Should have done that in the first place, but I forgot, sorry.
> While I was at it, I also prepare a solution to smooth out the whole
> localization process a bit. Once this patch lands, we can take a look
> at it, if you're interested.
>
Yes, definitely.  Anything to improve the codebase.

Thanks,
Om

On Fri, Aug 3, 2012 at 11:41 AM, Erik de Bruin <erik@ixsoftware.nl> wrote:

> > 1.  I see your comment about moving URL_CONFIG_XML constant from
> > ViewResourceConstants to the main class.  Resource constants dont
> > necessarily have to be localizable strings.  It can be urls, icons, sound
> > files etc.  I dont think such constants should be in the main class
> > itself.  IMHO it makes it inconsistent.
> > A good compromise would be place this constant in the
> ViewResourceConstants
> > class, but NOT load it as a locale string using iResourceManager .  Would
> > that be acceptable?
>
> I like compromises ;-) Done.
>
> > 2.  You have added a few log mesasges.  Can you please convert them into
> > locale strings for the en_US locale at least?  This will make it easier
> for
> > other locale contributors to provide corresponding translations.
>
> Done. Should have done that in the first place, but I forgot, sorry.
> While I was at it, I also prepare a solution to smooth out the whole
> localization process a bit. Once this patch lands, we can take a look
> at it, if you're interested.
>
> > 3.  When verifying the Apache Flex SDK MD5 signature, a message is being
> > logged for every progress update which causes a ton of log messages . You
> > probably can just log when the verification starts and when it ends.
>
> Done.
>
> > 4.  A log message about the result of the verification of the Apache Flex
> > SDK would be good.  Especially true when the md5 check fails and we
> abort,
> > we need to let the user know what just happened.
>
> Done.
>
> > 5.  Not a huge deal, but if possible, in the sequence of logs, can we
> make
> > sure that the Version number log is always the first one?
>
> Can you point me in the right direction, I can't find where it is
> logged at the moment...
>
> > 6.  For some reason, the 'temp' folder is not getting deleted after the
> > install.  This used to work.  Can you please debug and see if you
> > introduced something that causes this issue?
>
> I'm unable to reproduce... maybe it's a 'Windows' thing (I'm on a Mac)?
>
> > 7.  In the fetchApacheMirrorFromCGI() method, the result handler to
> > _internetUtil.fetch is specified to be the same method -
> > fetchApacheMirrorFromCGI()  This method is a bit too confusing.  I think
> it
> > will be better if you had a separate event handler where you process the
> > result and extract the url.
>
> Done.
>
> > 8.  Can you move all the mirror fetching logic into one class and
> provide a
> > common api?  The internal logic can be abstracted from the main class.
>
> Done.
>
> > I hope you dont mind fixing the patch based on these comments and
> > reattaching it when you get a chance.  I can then commit this patch to
> > svn.
>
> I've added the new patch to FLEX-33106.
>
> EdB
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
>
>
>
> P.S. I would like everybody to know that as of this morning I broke my
> personal record for days lived.
>

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