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

Issue 29323490: Issue 2751 - Add mozconfigs for devbuild/release builds (Closed)

Created:
Aug. 12, 2015, 9:55 p.m. by Felix Dahlke
Modified:
Aug. 13, 2015, 5:20 p.m.
Reviewers:
René Jeschke
Visibility:
Public.

Description

Issue 2751 - Add mozconfigs for devbuild/release builds

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -1 line) Patch
M .hgignore View 1 chunk +2 lines, -1 line 0 comments Download
M build.py View 1 chunk +4 lines, -0 lines 1 comment Download
A config.py.sample View 1 chunk +2 lines, -0 lines 0 comments Download
A mozconfig-devbuild View 1 chunk +9 lines, -0 lines 1 comment Download
A mozconfig-release View 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 2
Felix Dahlke
While this can be done prettier, I suppose this gives us something to start with. ...
Aug. 12, 2015, 10 p.m. (2015-08-12 22:00:14 UTC) #1
René Jeschke
Aug. 13, 2015, 8:15 a.m. (2015-08-13 08:15:05 UTC) #2
Just some comments, otherwise:

LGTM

https://codereview.adblockplus.org/29323490/diff/29323491/build.py
File build.py (right):

https://codereview.adblockplus.org/29323490/diff/29323491/build.py#newcode54
build.py:54: import config
I would have written that I dislike `import` in functions and recommended to
either import it globally as is, or as CONFIG, bus as you removed this in
another review anyway, I won't talk about this here.

https://codereview.adblockplus.org/29323490/diff/29323491/mozconfig-devbuild
File mozconfig-devbuild (right):

https://codereview.adblockplus.org/29323490/diff/29323491/mozconfig-devbuild#...
mozconfig-devbuild:9: export MOZILLA_OFFICIAL=1
Is there already a follow up issue (or the like) to make sure we get closer to
the configs that Mozilla uses? (I mean e.g. 
https://bitbucket.org/adblockplus/adblockbrowser/src/3ec6054b84fa223609d4ce43...
)

Maybe we can also extract a 'common' config and include that?

Just thinking out loud.

Powered by Google App Engine
This is Rietveld