[webkit-reviews] review canceled: [Bug 124848] Add support for multiple sources for AutoInstaller : [Attachment 218945] Replaced the mirror lists with iteratos.

[bugzilla-daemon at webkit.org]

Berta József <jberta.u-szeged at partner.samsung.com> has canceled Berta József
<jberta.u-szeged at partner.samsung.com>'s request for review:
Bug 124848: Add support for multiple sources for AutoInstaller
https://bugs.webkit.org/show_bug.cgi?id=124848

Attachment 218945: Replaced the mirror lists with iteratos.
https://bugs.webkit.org/attachment.cgi?id=218945&action=review

------- Additional Comments from Berta József
<jberta.u-szeged at partner.samsung.com>
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.