Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1917223003:
Disable whitespace/operators lint check. (Closed)
|
Created:
4 years, 8 months ago by kjellander_webrtc Modified:
4 years, 7 months ago CC:
webrtc-reviews_webrtc.org, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionDisable whitespace/operators lint check.
It gives false positives when using the move operator
(confuses them with logical And operators).
See http://crbug.com/464813 for more details.
NOTRY=True
Committed: https://crrev.com/e5a87a597443e633fa53c764bb23f66632f80c65
Cr-Commit-Position: refs/heads/master@{#12525}
Patch Set 1 #Patch Set 2 : Moved filter to a better place #Messages
Total messages: 17 (6 generated)
kjellander@webrtc.org changed reviewers: + perkj@webrtc.org
Description was changed from ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. ========== to ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. NOTRY=True ==========
kjellander@webrtc.org changed reviewers: + jbauch@webrtc.org
+jbauch: adding you since you created https://codereview.webrtc.org/1710293002. Per was running into errors like this in his https://codereview.webrtc.org/1900193004/. Were you seeing other false positives at the time you wrote your CL?
On 2016/04/27 08:48:30, kjellander (webrtc) wrote: > +jbauch: adding you since you created https://codereview.webrtc.org/1710293002. > Per was running into errors like this in his > https://codereview.webrtc.org/1900193004/. Were you seeing other false positives > at the time you wrote your CL? In my CL I only got the "build/c++11" false positive and I haven't seen any others since then.
lgtm I hope the real bug can be fixed.
On 2016/04/27 08:58:24, joachim wrote: > On 2016/04/27 08:48:30, kjellander (webrtc) wrote: > > +jbauch: adding you since you created > https://codereview.webrtc.org/1710293002. > > Per was running into errors like this in his > > https://codereview.webrtc.org/1900193004/. Were you seeing other false > positives > > at the time you wrote your CL? > > In my CL I only got the "build/c++11" false positive and I haven't seen any > others since then. Do you remember which CL or how the code looked like? Do you see anything obvious in Per's CL?
On 2016/04/27 09:01:18, kjellander (webrtc) wrote: > On 2016/04/27 08:58:24, joachim wrote: > > On 2016/04/27 08:48:30, kjellander (webrtc) wrote: > > > +jbauch: adding you since you created > > https://codereview.webrtc.org/1710293002. > > > Per was running into errors like this in his > > > https://codereview.webrtc.org/1900193004/. Were you seeing other false > > positives > > > at the time you wrote your CL? > > > > In my CL I only got the "build/c++11" false positive and I haven't seen any > > others since then. > > Do you remember which CL or how the code looked like? Do you see anything > obvious in Per's CL? It was https://codereview.webrtc.org/1697743003/ where I added a new class with a move ctor. I'll take a look at Per's CL shortly.
On 2016/04/27 09:05:42, joachim wrote: > On 2016/04/27 09:01:18, kjellander (webrtc) wrote: > > On 2016/04/27 08:58:24, joachim wrote: > > > On 2016/04/27 08:48:30, kjellander (webrtc) wrote: > > > > +jbauch: adding you since you created > > > https://codereview.webrtc.org/1710293002. > > > > Per was running into errors like this in his > > > > https://codereview.webrtc.org/1900193004/. Were you seeing other false > > > positives > > > > at the time you wrote your CL? > > > > > > In my CL I only got the "build/c++11" false positive and I haven't seen any > > > others since then. > > > > Do you remember which CL or how the code looked like? Do you see anything > > obvious in Per's CL? > > It was https://codereview.webrtc.org/1697743003/ where I added a new class with > a move ctor. > > I'll take a look at Per's CL shortly. I'm assuming the linter is confused by the "std::string&& encoder_name" parameter to some methods? Maybe it handles these different from being in a constructor (or in the same line as the rest of the method signature) but that's just guessing.
On 2016/04/27 09:15:07, joachim wrote: > On 2016/04/27 09:05:42, joachim wrote: > > On 2016/04/27 09:01:18, kjellander (webrtc) wrote: > > > On 2016/04/27 08:58:24, joachim wrote: > > > > On 2016/04/27 08:48:30, kjellander (webrtc) wrote: > > > > > +jbauch: adding you since you created > > > > https://codereview.webrtc.org/1710293002. > > > > > Per was running into errors like this in his > > > > > https://codereview.webrtc.org/1900193004/. Were you seeing other false > > > > positives > > > > > at the time you wrote your CL? > > > > > > > > In my CL I only got the "build/c++11" false positive and I haven't seen > any > > > > others since then. > > > > > > Do you remember which CL or how the code looked like? Do you see anything > > > obvious in Per's CL? > > > > It was https://codereview.webrtc.org/1697743003/ where I added a new class > with > > a move ctor. > > > > I'll take a look at Per's CL shortly. > > I'm assuming the linter is confused by the "std::string&& encoder_name" > parameter to some methods? > > Maybe it handles these different from being in a constructor (or in the same > line as the rest of the method signature) but that's just guessing. OK, let's disable this additional lint check for now then. I will track the Chromium bug for progress so we hopefully not just forget about this. I imagine there's a lot of other errors that we miss by disabling this one unfortunately.
The CQ bit was checked by kjellander@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1917223003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1917223003/20001
Message was sent while issue was closed.
Description was changed from ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. NOTRY=True ========== to ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. NOTRY=True ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. NOTRY=True ========== to ========== Disable whitespace/operators lint check. It gives false positives when using the move operator (confuses them with logical And operators). See http://crbug.com/464813 for more details. NOTRY=True Committed: https://crrev.com/e5a87a597443e633fa53c764bb23f66632f80c65 Cr-Commit-Position: refs/heads/master@{#12525} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/e5a87a597443e633fa53c764bb23f66632f80c65 Cr-Commit-Position: refs/heads/master@{#12525} |
