Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1704763002:
[GN] Add -Wno-unused-result on Linux and Android. (Closed)
|
Created:
4 years, 10 months ago by slan Modified:
4 years, 10 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/deps/icu.git@master Target Ref:
refs/heads/master Visibility:
Public. |
Description[GN] Add -Wno-unused-result on Linux and Android.
fread is used in putil.cpp without examining the return value. Suppress
this warning for ICU code only.
BUG=587965
TBR=brettw@chromium.org
Patch Set 1 #Messages
Total messages: 14 (5 generated)
Description was changed from ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only in order to build with uclibc. BUG=None ========== to ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only. BUG=None ==========
slan@chromium.org changed reviewers: + brettw@chromium.org, dpranke@chromium.org
+halliwell@ for FYI Dirk and Brett, we are getting an unused-result error here since M49. PTAL.
On 2016/02/17 01:40:49, slan wrote: > +halliwell@ for FYI > > Dirk and Brett, we are getting an unused-result error here since M49. PTAL. Did we file a bug to get this fixed upstream? This lgtm as a stop gap as long as we have a patch to getting this fixed properly (either upstream or via a patch instead).
On 2016/02/17 01:46:19, Dirk Pranke wrote: > On 2016/02/17 01:40:49, slan wrote: > > +halliwell@ for FYI > > > > Dirk and Brett, we are getting an unused-result error here since M49. PTAL. > > Did we file a bug to get this fixed upstream? This lgtm as a stop gap as long > as we have a patch to getting this fixed properly (either upstream or via a > patch > instead). Looks like it was fixed here: http://bugs.icu-project.org/trac/changeset/36903 What's the procedure? Do we just pick up the latest rev instead?
On 2016/02/17 20:18:05, slan wrote: > On 2016/02/17 01:46:19, Dirk Pranke wrote: > > On 2016/02/17 01:40:49, slan wrote: > > > +halliwell@ for FYI > > > > > > Dirk and Brett, we are getting an unused-result error here since M49. PTAL. > > > > Did we file a bug to get this fixed upstream? This lgtm as a stop gap as long > > as we have a patch to getting this fixed properly (either upstream or via a > > patch > > instead). > > Looks like it was fixed here: http://bugs.icu-project.org/trac/changeset/36903 > > What's the procedure? Do we just pick up the latest rev instead? Yes, normally someone (usually jshin@) will try to roll to a newer version of ICU. As you can see from README.chromium in that directory, this is not necessarily a trivial operation, so you might file a bug and ask jshin to take a look.
On 2016/02/18 00:11:18, Dirk Pranke wrote: > On 2016/02/17 20:18:05, slan wrote: > > On 2016/02/17 01:46:19, Dirk Pranke wrote: > > > On 2016/02/17 01:40:49, slan wrote: > > > > +halliwell@ for FYI > > > > > > > > Dirk and Brett, we are getting an unused-result error here since M49. > PTAL. > > > > > > Did we file a bug to get this fixed upstream? This lgtm as a stop gap as > long > > > as we have a patch to getting this fixed properly (either upstream or via a > > > patch > > > instead). > > > > Looks like it was fixed here: http://bugs.icu-project.org/trac/changeset/36903 > > > > What's the procedure? Do we just pick up the latest rev instead? > > Yes, normally someone (usually jshin@) will try to roll to a newer version of > ICU. > As you can see from README.chromium in that directory, this is not necessarily > a trivial operation, so you might file a bug and ask jshin to take a look. There has not been activity from jshin@ on crbug.com/587965. I would like to land this to unblock development in the meantime.
Description was changed from ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only. BUG=None ========== to ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only. BUG=None TBR=brettw@chromium.org ==========
slan@chromium.org changed reviewers: + jshin@chromium.org
Description was changed from ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only. BUG=None TBR=brettw@chromium.org ========== to ========== [GN] Add -Wno-unused-result on Linux and Android. fread is used in putil.cpp without examining the return value. Suppress this warning for ICU code only. BUG=587965 TBR=brettw@chromium.org ==========
On 2016/02/19 19:39:23, slan wrote: Yup, that's perfectly fine. I just wanted a follow-up somewhere.
On 2016/02/19 20:19:07, Dirk Pranke wrote: > On 2016/02/19 19:39:23, slan wrote: > > Yup, that's perfectly fine. I just wanted a follow-up somewhere. Thanks, Dirk. It's been taken care of between me and slan at https://codereview.chromium.org/1719453003/
On 2016/02/19 20:20:26, jshin (jungshik at google) wrote: > On 2016/02/19 20:19:07, Dirk Pranke wrote: > > On 2016/02/19 19:39:23, slan wrote: > > > > Yup, that's perfectly fine. I just wanted a follow-up somewhere. > > Thanks, Dirk. It's been taken care of between me and slan at > https://codereview.chromium.org/1719453003/ And DEPS roll will land soon: https://codereview.chromium.org/1710263003/ Closing this out. Thanks all! |
