Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29399743: Issue 3768 - Check (and fix, if needed) multilocale builds (Closed)

Created:
March 31, 2017, 9:34 a.m. by diegocarloslima
Modified:
April 4, 2017, 8:04 a.m.
Reviewers:
anton, Felix Dahlke
CC:
René Jeschke
Visibility:
Public.

Description

Issue 3768 - Check (and fix, if needed) multilocale builds Sibling of https://codereview.adblockplus.org/29374901/

Patch Set 1 #

Total comments: 9

Patch Set 2 : Removed unrelated change and updated ABB dependency #

Unified diffs Side-by-side diffs Delta from patch set Stats (+317 lines, -245 lines) Patch
M adblockbrowser-cfg.py View 1 chunk +2 lines, -2 lines 0 comments Download
M build.py View 1 2 chunks +6 lines, -4 lines 0 comments Download
M dependencies View 1 1 chunk +2 lines, -2 lines 0 comments Download
M ensure_dependencies.py View 1 chunk +307 lines, -237 lines 0 comments Download

Messages

Total messages: 9
diegocarloslima
March 31, 2017, 9:40 a.m. (2017-03-31 09:40:42 UTC) #1
Felix Dahlke
Looks good, just one nit. https://codereview.adblockplus.org/29399743/diff/29399744/build.py File build.py (right): https://codereview.adblockplus.org/29399743/diff/29399744/build.py#newcode156 build.py:156: "architecture", "architectures", _ARCHS) Seems ...
March 31, 2017, 9:53 a.m. (2017-03-31 09:53:54 UTC) #2
diegocarloslima
https://codereview.adblockplus.org/29399743/diff/29399744/build.py File build.py (right): https://codereview.adblockplus.org/29399743/diff/29399744/build.py#newcode156 build.py:156: "architecture", "architectures", _ARCHS) On 2017/03/31 09:53:54, Felix Dahlke wrote: ...
March 31, 2017, 10:07 a.m. (2017-03-31 10:07:23 UTC) #3
anton
https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py File adblockbrowser-cfg.py (right): https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py#newcode29 adblockbrowser-cfg.py:29: "ignore_locales": ["en-US", "en-ZA", "multi"], why "en-ZA" is ignored? https://codereview.adblockplus.org/29399743/diff/29399744/build.py ...
April 3, 2017, 6:28 a.m. (2017-04-03 06:28:11 UTC) #4
diegocarloslima
https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py File adblockbrowser-cfg.py (right): https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py#newcode29 adblockbrowser-cfg.py:29: "ignore_locales": ["en-US", "en-ZA", "multi"], On 2017/04/03 06:28:11, anton wrote: ...
April 3, 2017, 9:57 a.m. (2017-04-03 09:57:09 UTC) #5
anton
On 2017/04/03 09:57:09, diegocarloslima wrote: > https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py > File adblockbrowser-cfg.py (right): > > https://codereview.adblockplus.org/29399743/diff/29399744/adblockbrowser-cfg.py#newcode29 > ...
April 3, 2017, 10:18 a.m. (2017-04-03 10:18:47 UTC) #6
Felix Dahlke
LGTM On 2017/04/03 10:18:47, anton wrote: > LGTM though it's unclear why not to put ...
April 3, 2017, 4:26 p.m. (2017-04-03 16:26:36 UTC) #7
anton
On 2017/04/03 16:26:36, Felix Dahlke wrote: > LGTM > > On 2017/04/03 10:18:47, anton wrote: ...
April 4, 2017, 5:09 a.m. (2017-04-04 05:09:50 UTC) #8
Felix Dahlke
April 4, 2017, 8:04 a.m. (2017-04-04 08:04:46 UTC) #9
Message was sent while issue was closed.
On 2017/04/04 05:09:50, anton wrote:
> On 2017/04/03 16:26:36, Felix Dahlke wrote:
> > LGTM
> > 
> > On 2017/04/03 10:18:47, anton wrote:
> > > LGTM though it's unclear why not to put changes into
> > > buildtools/ensure_dependencies.py and then just use it.
> > 
> > You mean the ensure_dependencies.py changes? Those weren't actually made by
> > Diego, the script is copied to each repo and automatically updated whenever
> the
> > buildtools dependency gets updated.
> 
> Yes, i know but it's copied after the 1st run. But current
> ensure_dependencies.py isn't equal to the last version from buildtools repo
and
> this is misleading.

It is how I see it identical to revision 3816c08c0f98 of buildtools, which is
what the adblockbrowser->adblockplus->buildtools dependency chain refers to. If
we update adblockbrowser-build to point to an adblockbrowser revision that uses
an adblockplus revision that uses the latest revision of buildtools, the latest
version will get copied.

Powered by Google App Engine
This is Rietveld