Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
7 years, 8 months ago by danakj Modified:
7 years, 8 months ago Reviewers:
Elliot Glaysher, M-A Ruel CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, M-A Ruel, iannucci+depot_tools_chromium.org, piman, backer, jamesr, enne (OOO) Visibility:
Public. |
DescriptionTeach cpplint include-what-you-use about chromium hash_set and hash_map.
For hash_map and hash_set, we should include "base/hashtables.h", not
the <hash_set> or <hash_map> standard headers.
R=maruel@chromium.org
BUG=
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=194047
Patch Set 1 #
Total comments: 2
Patch Set 2 : #
Total comments: 2
Patch Set 3 : spaces #
Total comments: 2
Patch Set 4 : #
Created: 7 years, 8 months ago
Messages
Total messages: 12 (0 generated)
Sorry for the delay, I was travelling then sick. + Elliot to know if he approves this kind of surgery. https://codereview.chromium.org/14139002/diff/1/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/1/presubmit_canned_checks.py#ne... presubmit_canned_checks.py:122: else (a,b,header) I'm not a fan of the alignment, which makes it a tad hard to read; specifically the if is on the the same line but the else is else is at +2. So something like: cpplint._re_pattern_templates = [ (a, b, 'base/hash_tables.h') if header in ('<hash_map>', '<hash_set>') else (a, b, header) for a, b, header in cpplint._re_pattern_templates ]
On 2013/04/12 16:09:09, Marc-Antoine Ruel wrote: > Sorry for the delay, I was travelling then sick. > > + Elliot to know if he approves this kind of surgery. This sits inside the depot_tools repository, so yes.
https://codereview.chromium.org/14139002/diff/1/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/1/presubmit_canned_checks.py#ne... presubmit_canned_checks.py:122: else (a,b,header) On 2013/04/12 16:09:09, Marc-Antoine Ruel wrote: > I'm not a fan of the alignment, which makes it a tad hard to read; specifically > the if is on the the same line but the else is else is at +2. So something like: > > cpplint._re_pattern_templates = [ > (a, b, 'base/hash_tables.h') > if header in ('<hash_map>', '<hash_set>') else (a, b, header) > for a, b, header in cpplint._re_pattern_templates > ] Done.
https://codereview.chromium.org/14139002/diff/5001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/5001/presubmit_canned_checks.py... presubmit_canned_checks.py:120: (a,b,'base/hash_tables.h') we always keep space after a comma all the time
https://codereview.chromium.org/14139002/diff/5001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/5001/presubmit_canned_checks.py... presubmit_canned_checks.py:120: (a,b,'base/hash_tables.h') On 2013/04/12 20:28:51, Marc-Antoine Ruel wrote: > we always keep space after a comma all the time Ah right, of course. Done!
https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py... presubmit_canned_checks.py:121: if (header == '<hash_map>' or header == '<hash_set>') else (a, b, header) and why not as I proposed earlier? if header in ('<hash_map>', '<hash_set>') It's shorter.
On Fri, Apr 12, 2013 at 4:22 PM, <maruel@chromium.org> wrote: > > https://codereview.chromium.**org/14139002/diff/8001/** > presubmit_canned_checks.py<https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py> > File presubmit_canned_checks.py (right): > > https://codereview.chromium.**org/14139002/diff/8001/** > presubmit_canned_checks.py#**newcode121<https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py#newcode121> > presubmit_canned_checks.py:**121: if (header == '<hash_map>' or header == > > '<hash_set>') else (a, b, header) > and why not as I proposed earlier? > > if header in ('<hash_map>', '<hash_set>') > > It's shorter. > Ohh.. I just totally didn't see that in the diff, will do. > > https://codereview.chromium.**org/14139002/<https://codereview.chromium.org/1... >
https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/14139002/diff/8001/presubmit_canned_checks.py... presubmit_canned_checks.py:121: if (header == '<hash_map>' or header == '<hash_set>') else (a, b, header) On 2013/04/12 23:22:13, Marc-Antoine Ruel wrote: > and why not as I proposed earlier? > if header in ('<hash_map>', '<hash_set>') > > It's shorter. Done.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/14139002/13001
Message was sent while issue was closed.
Change committed as 194047 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
