|
|
DescriptionGetting 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 #
Messages
Total messages: 31 (22 generated)
romax@chromium.org changed reviewers: + dcheng@chromium.org
can you please take a look? referring to this line of TODO: https://cs.chromium.org/chromium/src/base/sys_info.h?rcl=1506f35e2a82f1b50ba2... Thanks!
Description was changed from ========== Getting OS version numbers for Linux. Implementing the method to get OS version numbers for Linux and enabled corresponding unit test. BUG=NONE ========== to ========== Getting OS version numbers for Linux. Implementing the method to get OS version numbers for Linux and enabled corresponding unit test. BUG=NONE ==========
romax@chromium.org changed reviewers: + petewil@chromium.org
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#newc... base/sys_info_posix.cc:199: sscanf(info.release, "%d.%d.%d", major_version, minor_version, Is the formatting of |release| guaranteed by some standard?
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#newc... 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: > Is the formatting of |release| guaranteed by some standard? Also, I'd probably prefer the approach that iOS uses: https://cs.chromium.org/chromium/src/base/sys_info_ios.mm?rcl=60d92c2f3ff60c2...
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#newc... base/sys_info_posix.cc:199: sscanf(info.release, "%d.%d.%d", major_version, minor_version, On 2017/03/21 06:53:37, dcheng wrote: > On 2017/03/21 06:51:17, dcheng wrote: > > Is the formatting of |release| guaranteed by some standard? > > Also, I'd probably prefer the approach that iOS uses: > https://cs.chromium.org/chromium/src/base/sys_info_ios.mm?rcl=60d92c2f3ff60c2... Tried a search online and didn't find a standard format for |release|. Also moved the '=0's into the error case calling uname(&info), is it reasonable to have default values here?
lgtm
The CQ bit was checked by romax@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
The CQ bit was checked by romax@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity)
The CQ bit was checked by romax@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2764593002/#ps40001 (title: "Fixing build")
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
Failed to apply patch for base/sys_info_unittest.cc: While running git apply --index -p1; error: patch failed: base/sys_info_unittest.cc:45 error: base/sys_info_unittest.cc: patch does not apply Patch: base/sys_info_unittest.cc Index: base/sys_info_unittest.cc diff --git a/base/sys_info_unittest.cc b/base/sys_info_unittest.cc index c3b85077071a8ead6a263947fc109e374516a761..e3592bf28b07a11cb6076ac37c3cccea0c4b546f 100644 --- a/base/sys_info_unittest.cc +++ b/base/sys_info_unittest.cc @@ -45,7 +45,7 @@ TEST_F(SysInfoTest, AmountOfTotalDiskSpace) { << tmp_path.value(); } -#if defined(OS_WIN) || defined(OS_MACOSX) +#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) TEST_F(SysInfoTest, OperatingSystemVersionNumbers) { int32_t os_major_version = -1; int32_t os_minor_version = -1;
The CQ bit was checked by romax@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...
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 romax@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2764593002/#ps60001 (title: "Fixing build")
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": 1490212346312350, "parent_rev": "dac09c03b9f3cc44adf71e8836f6bebabec7a61d", "commit_rev": "2cd67b5b3f57bd69c5a88e927852600f7b956912"}
Message was sent while issue was closed.
Description was changed from ========== Getting OS version numbers for Linux. Implementing the method to get OS version numbers for Linux and enabled corresponding unit test. BUG=NONE ========== to ========== 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/+/2cd67b5b3f57bd69c5a88e927852... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/2cd67b5b3f57bd69c5a88e927852... |