Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(16)

Issue 2764593002: Getting OS version numbers for Linux. (Closed)

Created:
2 years, 8 months ago by romax
Modified:
2 years, 8 months ago
Reviewers:
Pete Williamson, dcheng
CC:
chromium-reviews, vmpstr+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Getting OS version numbers for Linux. Implementing the method to get OS version numbers for Linux and enabled corresponding unit test. BUG=NONE Review-Url: https://codereview.chromium.org/2764593002 Cr-Commit-Position: refs/heads/master@{#458854} Committed: https://chromium.googlesource.com/chromium/src/+/2cd67b5b3f57bd69c5a88e927852600f7b956912

Patch Set 1 #

Total comments: 3

Patch Set 2 : comments. #

Patch Set 3 : Fixing build #

Patch Set 4 : Fixing build #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -3 lines) Patch
M base/sys_info.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M base/sys_info_posix.cc View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
M base/sys_info_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 31 (22 generated)
romax
can you please take a look? referring to this line of TODO: https://cs.chromium.org/chromium/src/base/sys_info.h?rcl=1506f35e2a82f1b50ba2aa51e61c60891dc8cbcc&l=73 Thanks!
2 years, 8 months ago (2017-03-20 20:11:49 UTC) #2
dcheng
https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc File base/sys_info_posix.cc (right): https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc#newcode199 base/sys_info_posix.cc:199: sscanf(info.release, "%d.%d.%d", major_version, minor_version, Is the formatting of |release| ...
2 years, 8 months ago (2017-03-21 06:51:17 UTC) #5
dcheng
https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc File base/sys_info_posix.cc (right): https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc#newcode199 base/sys_info_posix.cc:199: sscanf(info.release, "%d.%d.%d", major_version, minor_version, On 2017/03/21 06:51:17, dcheng wrote: ...
2 years, 8 months ago (2017-03-21 06:53:38 UTC) #6
romax
thanks for the comments, PTAnL. https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc File base/sys_info_posix.cc (right): https://codereview.chromium.org/2764593002/diff/1/base/sys_info_posix.cc#newcode199 base/sys_info_posix.cc:199: sscanf(info.release, "%d.%d.%d", major_version, minor_version, ...
2 years, 8 months ago (2017-03-21 18:17:33 UTC) #7
dcheng
lgtm
2 years, 8 months ago (2017-03-22 01:01:38 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2764593002/40001
2 years, 8 months ago (2017-03-22 17:30:34 UTC) #19
commit-bot: I haz the power
Failed to apply patch for base/sys_info_unittest.cc: While running git apply --index -p1; error: patch failed: ...
2 years, 8 months ago (2017-03-22 17:34:36 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2764593002/60001
2 years, 8 months ago (2017-03-22 19:53:13 UTC) #28
commit-bot: I haz the power
2 years, 8 months ago (2017-03-22 20:02:34 UTC) #31
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/2cd67b5b3f57bd69c5a88e927852...

Powered by Google App Engine
This is Rietveld 408576698