Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2893313003:
Add a switch to prevent logging redirection on chromeos. (Closed)
DescriptionAdd a switch to prevent logging redirection on chromeos.
This switch will be used by telemetry so all chrome logging goes to
/var/log/chrome/chrome instead of split between this location and
/home/chronos/user/log/chrome, which is in the cryptohome, and sometimes
not collected by autotests.
BUG=chromium:724273
TEST=manual
Review-Url: https://codereview.chromium.org/2893313003
Cr-Commit-Position: refs/heads/master@{#476476}
Committed: https://chromium.googlesource.com/chromium/src/+/279874b4b774288861a5bda6934fa6a7e3fc5153
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix alpha ordering #Patch Set 3 : Rebase #Patch Set 4 : Fix include #
Messages
Total messages: 31 (18 generated)
The CQ bit was checked by achuith@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
achuith@chromium.org changed reviewers: + xiyuan@chromium.org
PTAL, Xiyuan
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by achuith@chromium.org
The CQ bit was unchecked by achuith@chromium.org
The CQ bit was checked by achuith@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
achuith@chromium.org changed reviewers: + sky@chromium.org
Scott, could I get an owner lgtm for logging_chrome.cc?
https://codereview.chromium.org/2893313003/diff/1/chrome/common/chrome_switch... File chrome/common/chrome_switches.h (right): https://codereview.chromium.org/2893313003/diff/1/chrome/common/chrome_switch... chrome/common/chrome_switches.h:275: extern const char kDisableLoggingRedirect[]; keep sorted. https://codereview.chromium.org/2893313003/diff/1/chrome/common/logging_chrom... File chrome/common/logging_chrome.cc (right): https://codereview.chromium.org/2893313003/diff/1/chrome/common/logging_chrom... chrome/common/logging_chrome.cc:231: void RedirectChromeLogging(const base::CommandLine& command_line) { AFAICT this function is only used in chromeos. Code that lives in chrome/common is meant to be used both in the browser and renderer. This code seems to only be used in the browser. Can this function be moved to chromeos specific code?
The CQ bit was checked by achuith@chromium.org to run a CQ dry run
PTAL Xiyuan and Scott https://codereview.chromium.org/2893313003/diff/1/chrome/common/chrome_switch... File chrome/common/chrome_switches.h (right): https://codereview.chromium.org/2893313003/diff/1/chrome/common/chrome_switch... chrome/common/chrome_switches.h:275: extern const char kDisableLoggingRedirect[]; On 2017/05/23 17:35:31, sky wrote: > keep sorted. Done. https://codereview.chromium.org/2893313003/diff/1/chrome/common/logging_chrom... File chrome/common/logging_chrome.cc (right): https://codereview.chromium.org/2893313003/diff/1/chrome/common/logging_chrom... chrome/common/logging_chrome.cc:231: void RedirectChromeLogging(const base::CommandLine& command_line) { On 2017/05/23 17:35:31, sky wrote: > AFAICT this function is only used in chromeos. Code that lives in chrome/common > is meant to be used both in the browser and renderer. This code seems to only be > used in the browser. Can this function be moved to chromeos specific code? Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
still lgtm
On 2017/05/25 19:38:36, xiyuan wrote: > still lgtm Ty! Will land by EOD if I don't hear from Scott.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
LGTM
Rebased, landing based on prior lgtms
The CQ bit was checked by achuith@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/2893313003/#ps60001 (title: "Fix include")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496352502689470,
"parent_rev": "64ccc57c1a20a8b2804df8a4e9be42b33b5b8f61", "commit_rev":
"279874b4b774288861a5bda6934fa6a7e3fc5153"}
Message was sent while issue was closed.
Description was changed from ========== Add a switch to prevent logging redirection on chromeos. This switch will be used by telemetry so all chrome logging goes to /var/log/chrome/chrome instead of split between this location and /home/chronos/user/log/chrome, which is in the cryptohome, and sometimes not collected by autotests. BUG=chromium:724273 TEST=manual ========== to ========== Add a switch to prevent logging redirection on chromeos. This switch will be used by telemetry so all chrome logging goes to /var/log/chrome/chrome instead of split between this location and /home/chronos/user/log/chrome, which is in the cryptohome, and sometimes not collected by autotests. BUG=chromium:724273 TEST=manual Review-Url: https://codereview.chromium.org/2893313003 Cr-Commit-Position: refs/heads/master@{#476476} Committed: https://chromium.googlesource.com/chromium/src/+/279874b4b774288861a5bda6934f... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/279874b4b774288861a5bda6934f... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
