Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Issue 2824008: Command line flag cleanup. (Closed)

Created:
10 years, 6 months ago by Evan Stade
Modified:
9 years, 6 months ago
Reviewers:
Robert Sesek
CC:
chromium-reviews, ncarter (slow), idana, ben+cc_chromium.org, pam+watch_chromium.org, davemoore+watch_chromium.org, Paweł Hajdan Jr., tim (not reviewing)
Base URL:
http://src.chromium.org/git/chromium.git
Visibility:
Public.

Description

Command line flag cleanup. Move some string literals into chrome/common/chrome_switches. There are still many uses in places that cannot depend on chrome/common. BUG=none TEST=trybots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50888

Patch Set 1 #

Patch Set 2 : fix compile? #

Patch Set 3 : include common_constants #

Total comments: 4

Patch Set 4 : fix gyp #

Patch Set 5 : gyp fix #

Patch Set 6 : win compile #

Total comments: 2

Patch Set 7 : copyright years #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -26 lines) Patch
M chrome/app/chrome_dll_main.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/input_method/candidate_window.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sync/tools/sync_listen_notifications.cc View 1 2 3 4 5 chunks +13 lines, -9 lines 0 comments Download
M chrome/browser/sync/tools/sync_tools.gyp View 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 4 chunks +17 lines, -2 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 chunks +48 lines, -4 lines 0 comments Download
M chrome/test/mini_installer_test/run_all_unittests.cc View 1 2 3 4 5 6 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/test/mini_installer_test/test.cc View 1 2 3 4 5 6 2 chunks +6 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Evan Stade
10 years, 6 months ago (2010-06-18 20:11:56 UTC) #1
Robert Sesek
http://codereview.chromium.org/2824008/diff/4001/5004 File chrome/browser/sync/tools/sync_tools.gyp (right): http://codereview.chromium.org/2824008/diff/4001/5004#newcode24 chrome/browser/sync/tools/sync_tools.gyp:24: '<(DEPTH)/chrome/common_constants.gypi', This needs to pull in a target. I ...
10 years, 6 months ago (2010-06-18 20:19:13 UTC) #2
Evan Stade
http://codereview.chromium.org/2824008/diff/4001/5004 File chrome/browser/sync/tools/sync_tools.gyp (right): http://codereview.chromium.org/2824008/diff/4001/5004#newcode24 chrome/browser/sync/tools/sync_tools.gyp:24: '<(DEPTH)/chrome/common_constants.gypi', On 2010/06/18 20:19:13, rsesek wrote: > This needs ...
10 years, 6 months ago (2010-06-18 20:25:45 UTC) #3
Robert Sesek
So the changes overall look good, but the trybots are still choking on the GYP. ...
10 years, 6 months ago (2010-06-21 18:51:48 UTC) #4
Evan Stade
fixed the gyp problem. common_constants is included in chrome_browser.gyp, so the correct incantation for the ...
10 years, 6 months ago (2010-06-24 20:00:22 UTC) #5
Robert Sesek
10 years, 6 months ago (2010-06-24 20:20:56 UTC) #6
LGTM pending trybot success.

http://codereview.chromium.org/2824008/diff/17001/18007
File chrome/test/mini_installer_test/run_all_unittests.cc (right):

http://codereview.chromium.org/2824008/diff/17001/18007#newcode1
chrome/test/mini_installer_test/run_all_unittests.cc:1: // Copyright (c) 2009
The Chromium Authors. All rights reserved.
2010

http://codereview.chromium.org/2824008/diff/17001/18008
File chrome/test/mini_installer_test/test.cc (right):

http://codereview.chromium.org/2824008/diff/17001/18008#newcode1
chrome/test/mini_installer_test/test.cc:1: // Copyright (c) 2009 The Chromium
Authors. All rights reserved.
2010

Powered by Google App Engine
This is Rietveld 408576698