|
|
Created:
4 years ago by sdefresne Modified:
4 years ago Reviewers:
Dirk Pranke CC:
chromium-reviews, tfarina, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionClean formatting after http://crrev.com/2544803002/.
As recommended, reformat the array definition (locally disable
clang-format) and use range-base loop (as it works with plain
arrays).
BUG=None
Committed: https://crrev.com/cdeaacd4acb0467526ea9a4ce27859f731417b6c
Cr-Commit-Position: refs/heads/master@{#436902}
Patch Set 1 #
Messages
Total messages: 18 (8 generated)
The CQ bit was checked by sdefresne@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...
sdefresne@chromium.org changed reviewers: + dpranke@chromium.org
Please take a look and send to CQ if LGTY.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm. If clang-format produced the prior formatting, that looks like a bug to me. Mind filing one and cc'ing thakis@ and nick@ ?
The CQ bit was checked by sdefresne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/06 20:16:34, Dirk Pranke wrote: > lgtm. If clang-format produced the prior formatting, that looks like a bug to > me. Mind filing one and cc'ing thakis@ and nick@ ? Issue reported (you are in cc: with thakis@ but I could not find nick@)
CQ is committing da patch. Bot data: {"patchset_id": 1, "attempt_start_ts": 1481104108275590, "parent_rev": "721e076a94305cd8a628ee9e3195d3e9833e6502", "commit_rev": "d5b1125f274af94cb5f034a443fbd0a65fe0a97d"}
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Clean formatting after http://crrev.com/2544803002/. As recommended, reformat the array definition (locally disable clang-format) and use range-base loop (as it works with plain arrays). BUG=None ========== to ========== Clean formatting after http://crrev.com/2544803002/. As recommended, reformat the array definition (locally disable clang-format) and use range-base loop (as it works with plain arrays). BUG=None Committed: https://crrev.com/cdeaacd4acb0467526ea9a4ce27859f731417b6c Cr-Commit-Position: refs/heads/master@{#436902} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/cdeaacd4acb0467526ea9a4ce27859f731417b6c Cr-Commit-Position: refs/heads/master@{#436902}
Message was sent while issue was closed.
On 2016/12/06 20:16:34, Dirk Pranke wrote: > lgtm. If clang-format produced the prior formatting, that looks like a bug to > me. Mind filing one and cc'ing thakis@ and nick@ ? Is the bug the excess of spaces, or that it puts more than one of these on one line? If the former, that does seem like the bug. If the latter, I'd like to argue that it isn't.
Message was sent while issue was closed.
On 2016/12/07 15:29:23, Nico wrote: > Is the bug the excess of spaces, or that it puts more than one of these on one > line? If the former, that does seem like the bug. If the latter, I'd like to > argue that it isn't. The excess spaces.
Message was sent while issue was closed.
On 2016/12/07 15:38:01, Dirk Pranke wrote: > On 2016/12/07 15:29:23, Nico wrote: > > Is the bug the excess of spaces, or that it puts more than one of these on one > > line? If the former, that does seem like the bug. If the latter, I'd like to > > argue that it isn't. > > The excess spaces. Per internal bug, that's because clang-format tries to align columns, even if there's only one row with aligned columns. I agree that this looks weird, and it sounds like it will be fixed. Having said that, I don't think this warrants a clang-format off override. Adding this has a real cost: If someone does an automatic refactoring, they either have to manually update this block, or (more likely) they won't think of having to check for these blocks and it'll look worse than without the clang-format off block after the rewriting. So I'd recommend reverting this.
Message was sent while issue was closed.
Agreed, I'm fine with (and prefer) weird formatting if there's a clang-format bug on file for it. |