[Webkit-unassigned] [Bug 124848] Add support for multiple sources for AutoInstaller

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=124848


Berta József <jberta.u-szeged at partner.samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #218945|review?, commit-queue?      |
               Flag|                            |




--- Comment #9 from Berta József <jberta.u-szeged at partner.samsung.com>  2014-01-08 07:26:33 PST ---
(From update of attachment 218945)
View in context: https://bugs.webkit.org/attachment.cgi?id=218945&action=review

>> Tools/Scripts/webkitpy/common/system/autoinstall.py:310
>> +        Mirror sources: http://www.pypi-mirrors.org/, http://sourceforge.net/apps/trac/sourceforge/wiki/Mirrors
> 
> This is an excessive amount of comments/examples for a very simple example.
> 
> It's much better to convey this information via a more descriptive function name such as
> parse_colon_separated_mirrors_from_env

Ok, I removed most of it, but the rest is necessary, i think

>> Tools/Scripts/webkitpy/common/system/autoinstall.py:327
>> +        parsed_url = list(urlparse.urlparse(url))
> 
> Why are we converting the parsed result into a list?  That's reducing the readability of the code below.

The urlparse() returns a tuple, which can't be modified.

>> Tools/Scripts/webkitpy/common/system/autoinstall.py:388
>> +                return path[0]
> 
> Why are we returning the first path?

Glob returns a list, in this case containing only one string. We only need the string, not a list.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.