[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





--- Comment #5 from Ryosuke Niwa <rniwa at webkit.org>  2013-12-09 10:49:18 PST ---
(From update of attachment 218775)
View in context: https://bugs.webkit.org/attachment.cgi?id=218775&action=review

> Tools/Scripts/webkitpy/common/system/autoinstall.py:54
> +_mirror_regexs = re.compile('.*sourceforge.*'), re.compile('.*pypi.*')
> +_pypi_env_var = 'PYPI_MIRRORS'
> +_sourceforge_env_var = 'SOURCEFORGE_MIRRORS'
> +_cache_env_var = 'LOCAL_AUTOINSTALL_CACHE'

I think we normally use ALL CAPS for constants.

> Tools/Scripts/webkitpy/common/system/autoinstall.py:332
> +            if regex.match(parsed_url[1]):
> +                if mirror_index < len(addresses):

Combine these two ifs.

> Tools/Scripts/webkitpy/common/system/autoinstall.py:355
> +                    url = self._replace_domain_with_next_mirror(url, mirrors, mirror_index)
> +                    if url:

Why don't we just pop the next item from mirrors instead of keeping mirror_index separately?

> Tools/Scripts/webkitpy/common/system/autoinstall.py:385
> +        #The cache must contain the packages in their packed forms(As if they were just downloaded from the web)
> +
> +        #The path of the cache is stored in the environment variable named in the _cache_env_var variable.

Nit: You should put a space between # and The.
Nit: You also need a space between forms and (, and "As" should not be capitalized and ) should be followed by a single period.

> Tools/Scripts/webkitpy/common/system/autoinstall.py:398
> +        #The installer first looks at the local cache, and if the package is found there, it will be installed from there.

I don't think this comment is necessary.

> Tools/Scripts/webkitpy/common/system/autoinstall.py:407
> +        # If this part of the code is reached, there is no copy of the package in the local cache, so it will be copied there.

Ditto.

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