Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2648263005:
Run software reporter's experimental engine on all architectures (Closed)
DescriptionRun software reporter's experimental engine on all architectures
Allows using the experimental engine of the software reporter component
if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled
regardless of the system's bitness.
BUG=685705
Review-Url: https://codereview.chromium.org/2648263005
Cr-Commit-Position: refs/heads/master@{#446127}
Committed: https://chromium.googlesource.com/chromium/src/+/8f32a8905fcbc1c904ac48add7e66696cba2ddc0
Patch Set 1 #
Total comments: 3
Messages
Total messages: 17 (9 generated)
Description was changed from ========== Run experimental engine on x64 BUG= ========== to ========== Run software reporter's experimental engine on all platforms Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled. BUG= ==========
Description was changed from ========== Run software reporter's experimental engine on all platforms Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled. BUG= ========== to ========== Run software reporter's experimental engine on all architectures Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled regardless of the system's bitness. BUG= ==========
veranika@chromium.org changed reviewers: + joenotcharles@chromium.org
lgtm https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... File chrome/browser/component_updater/sw_reporter_installer_win.cc (right): https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... chrome/browser/component_updater/sw_reporter_installer_win.cc:490: is_x86_architecture; Nit: I don't think the extra "is_x86_architecture" variable name adds any clarity here. I'd just use ||.
https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... File chrome/browser/component_updater/sw_reporter_installer_win.cc (right): https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... chrome/browser/component_updater/sw_reporter_installer_win.cc:490: is_x86_architecture; On 2017/01/24 21:06:14, Joe Mason wrote: > Nit: I don't think the extra "is_x86_architecture" variable name adds any > clarity here. I'd just use ||. Acknowledged. I agree that it isn't necessary, but because of the variable length the inlined expression is harder to understand, and using parenthesis would be ugly: base::FeatureList::IsEnabled(kExperimentalEngineAllArchsFeature) || base::win::OSInfo::GetInstance()->architecture() == base::win::OSInfo::X86_ARCHITECTURE;
joenotcharles@google.com changed reviewers: + joenotcharles@google.com
https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... File chrome/browser/component_updater/sw_reporter_installer_win.cc (right): https://codereview.chromium.org/2648263005/diff/1/chrome/browser/component_up... chrome/browser/component_updater/sw_reporter_installer_win.cc:490: is_x86_architecture; On 2017/01/25 16:12:59, veranika wrote: > On 2017/01/24 21:06:14, Joe Mason wrote: > > Nit: I don't think the extra "is_x86_architecture" variable name adds any > > clarity here. I'd just use ||. > Acknowledged. > I agree that it isn't necessary, but because of the variable length the inlined > expression is harder to understand, and using parenthesis would be ugly: > > base::FeatureList::IsEnabled(kExperimentalEngineAllArchsFeature) || > base::win::OSInfo::GetInstance()->architecture() == > base::win::OSInfo::X86_ARCHITECTURE; Acknowledged.
veranika@chromium.org changed reviewers: + sorin@chromium.org
R += sorin
lgtm Thank you!
The CQ bit was checked by veranika@chromium.org
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": 1, "attempt_start_ts": 1485366298900640, "parent_rev":
"776f9dbea5f6be3db4722fd21405151545a008eb", "commit_rev":
"8f32a8905fcbc1c904ac48add7e66696cba2ddc0"}
Message was sent while issue was closed.
Description was changed from ========== Run software reporter's experimental engine on all architectures Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled regardless of the system's bitness. BUG= ========== to ========== Run software reporter's experimental engine on all architectures Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled regardless of the system's bitness. BUG= Review-Url: https://codereview.chromium.org/2648263005 Cr-Commit-Position: refs/heads/master@{#446127} Committed: https://chromium.googlesource.com/chromium/src/+/8f32a8905fcbc1c904ac48add7e6... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/8f32a8905fcbc1c904ac48add7e6...
Message was sent while issue was closed.
Description was changed from ========== Run software reporter's experimental engine on all architectures Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled regardless of the system's bitness. BUG= Review-Url: https://codereview.chromium.org/2648263005 Cr-Commit-Position: refs/heads/master@{#446127} Committed: https://chromium.googlesource.com/chromium/src/+/8f32a8905fcbc1c904ac48add7e6... ========== to ========== Run software reporter's experimental engine on all architectures Allows using the experimental engine of the software reporter component if ExperimentalSwReporterEngineOnAllArchitectures feature is enabled regardless of the system's bitness. BUG=685705 Review-Url: https://codereview.chromium.org/2648263005 Cr-Commit-Position: refs/heads/master@{#446127} Committed: https://chromium.googlesource.com/chromium/src/+/8f32a8905fcbc1c904ac48add7e6... ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
