Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

R=dbeam@chromium.org,maruel@chromium.org
BUG=545272
Committed: https://crrev.com/c430ad4e9110a195902c7b8e886026e8d8353327
Cr-Commit-Position: refs/heads/master@{#355639}

[oystein (OOO til 10th of July), 2015-10-19 21:06:59]

Description was changed from

==========
Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

R=dbeam@chromium.org,cpu@chromium.org,maruel@chromium.org
==========

to

==========
Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

R=dbeam@chromium.org,maruel@chromium.org
==========

[oystein (OOO til 10th of July), 2015-10-19 21:06:59]

oysteine@chromium.org changed reviewers:
- cpu@chromium.org

[oystein (OOO til 10th of July), 2015-10-19 21:07:22]

ptal!

[Dan Beam, 2015-10-19 22:19:36]

https://codereview.chromium.org/1413263002/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1413263002/diff/1/PRESUBMIT.py#newcode1506
PRESUBMIT.py:1506: if (not input_api.re.match(r'//', line) and # Strip C++
comment.
this doesn't work that well, IMO, unless I'm missing some magic stripping
somewhere (also, striplines() doesn't strip extra \s, just \n or \r\n, afaik)

$ python
>>> import re
>>> re.match(r'//', '  // Example code') 
>>> re.search(r'//', '  // Example code')
<_sre.SRE_Match object at 0x7f90a5ad7ac0>

https://codereview.chromium.org/1413263002/diff/1/PRESUBMIT.py#newcode1532
PRESUBMIT.py:1532: if input_api.re.match(r'//', line): # Strip C++ comment.
this would only match lines that start with "//" and no spaces (I'm pretty
sure).  that's why I didn't port it from the other check.  alternatives:

  if input_api.re.search(r'//', line):  # Strip C++ comment.

(match -> search) OR

  if line.lstrip().startswith('//'):  # Strip C++ comment.

(also note: \s\s# instead of \s#)

personally prefer second line

also maybe update the other check?

[Dan Beam, 2015-10-19 22:20:36]

also, please update CheckBaseMacrosInHeadersTest

[oystein (OOO til 10th of July), 2015-10-20 00:04:05]

https://codereview.chromium.org/1413263002/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1413263002/diff/1/PRESUBMIT.py#newcode1532
PRESUBMIT.py:1532: if input_api.re.match(r'//', line): # Strip C++ comment.
On 2015/10/19 22:19:36, Dan Beam wrote:
> this would only match lines that start with "//" and no spaces (I'm pretty
> sure).  that's why I didn't port it from the other check.  alternatives:
> 
>   if input_api.re.search(r'//', line):  # Strip C++ comment.
> 
> (match -> search) OR
> 
>   if line.lstrip().startswith('//'):  # Strip C++ comment.
> 
> (also note: \s\s# instead of \s#)
> 
> personally prefer second line
> 
> also maybe update the other check?

Good catch; I just blindly copied the check from the other test. Done.

[Dan Beam, 2015-10-20 00:51:56]

lgtm

[Dan Beam, 2015-10-20 00:59:03]

Description was changed from

==========
Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

R=dbeam@chromium.org,maruel@chromium.org
==========

to

==========
Changed the CheckBaseMacrosInHeaders presubmit to pass over C++ comments

R=dbeam@chromium.org,maruel@chromium.org
BUG=545272
==========

[Dan Beam, 2015-10-20 00:59:15]

filed a bug and added BUG= so others can search for this if they hit it

[oystein (OOO til 10th of July), 2015-10-21 18:15:11]

maruel: ping :)

On 2015/10/20 00:59:15, Dan Beam wrote:
> filed a bug and added BUG= so others can search for this if they hit it

Perfect, thanks!

[M-A Ruel, 2015-10-21 18:16:23]

lgtm

[oystein (OOO til 10th of July), 2015-10-21 18:20:30]

The CQ bit was checked by oysteine@chromium.org

[commit-bot: I haz the power, 2015-10-21 18:21:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413263002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413263002/60001

[commit-bot: I haz the power, 2015-10-21 18:38:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 18:38:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[oystein (OOO til 10th of July), 2015-10-21 19:17:13]

oysteine@chromium.org changed reviewers:
+ jochen@chromium.org

[oystein (OOO til 10th of July), 2015-10-21 19:17:47]

+jochen for /PRESUBMIT*.py owner; ptal :)

[jochen (gone - plz use gerrit), 2015-10-22 12:06:58]

lgtm

[oystein (OOO til 10th of July), 2015-10-22 20:43:09]

The CQ bit was checked by oysteine@chromium.org

[commit-bot: I haz the power, 2015-10-22 20:44:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413263002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413263002/60001

[commit-bot: I haz the power, 2015-10-22 20:55:32]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-22 20:56:07]

Patchset 4 (id:??) landed as
https://crrev.com/c430ad4e9110a195902c7b8e886026e8d8353327
Cr-Commit-Position: refs/heads/master@{#355639}