Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(322)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 9 months ago by diegocarloslima
Modified:
2 years, 9 months ago
Reviewers:
Felix Dahlke, anton
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
2 years, 9 months ago (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 ...
2 years, 9 months ago (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: ...
2 years, 9 months ago (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 ...
2 years, 9 months ago (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: ...
2 years, 9 months ago (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 > ...
2 years, 9 months ago (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 ...
2 years, 9 months ago (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: ...
2 years, 9 months ago (2017-04-04 05:09:50 UTC) #8
Felix Dahlke
2 years, 9 months ago (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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5