Enforce clang-format in //base

Enforce clang-format in //base

BUG=none

Committed: https://crrev.com/c2f1eb57956525246abe3800fad1bfd672d99d14
Cr-Commit-Position: refs/heads/master@{#431766}

[dcheng, 2016-11-10 23:04:09]

Description was changed from

==========
//base: clang-format source and enforce formatting.

clang-format -i --style=file $(git ls-files | grep -E "(\\.cc|\\.h|\\.mm)$" |
grep -v third_party)

BUG=
==========

to

==========
Enforce clang-format in //base

BUG=none
==========

[dcheng, 2016-11-10 23:04:09]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org, mark@chromium.org, thakis@chromium.org,
thestig@chromium.org

[dcheng, 2016-11-10 23:05:27]

Any thoughts?

Originally, I clang-formatted //base, but that's a 15k lines changed type of
patch. I think we can just enforce clang-formatting and allow gradual
convergence over time.

[danakj, 2016-11-10 23:06:09]

LGTM I also welcome clang-format-only patches on all of base or on subdirs.

[danakj, 2016-11-10 23:06:51]

https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py
File base/PRESUBMIT.py (right):

https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py#newco...
base/PRESUBMIT.py:39: input_api.canned_checks.CheckPatchFormatted(input_api,
output_api))
You might want this only on upload though? I'm not sure. At least I've seen it
that way in other places, usually I don't see CheckChangeOnCommit anymore at
all.. CQ life?

[Nico, 2016-11-10 23:11:04]

lgtm, makes sense to me.

[dcheng, 2016-11-10 23:49:58]

On 2016/11/10 23:06:51, danakj wrote:
> https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py
> File base/PRESUBMIT.py (right):
> 
>
https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py#newco...
> base/PRESUBMIT.py:39: input_api.canned_checks.CheckPatchFormatted(input_api,
> output_api))
> You might want this only on upload though? I'm not sure. At least I've seen it
> that way in other places, usually I don't see CheckChangeOnCommit anymore at
> all.. CQ life?

Hm... doesn't that mean a directly committed patch would skip the formatting
check? (assuming --bypass-hooks wasn't specified)

[danakj, 2016-11-10 23:56:02]

On Thu, Nov 10, 2016 at 3:49 PM, <dcheng@chromium.org> wrote:

> On 2016/11/10 23:06:51, danakj wrote:
> > https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py
> > File base/PRESUBMIT.py (right):
> >
> >
> https://codereview.chromium.org/2489153003/diff/10001/
> base/PRESUBMIT.py#newcode39
> > base/PRESUBMIT.py:39: input_api.canned_checks.
> CheckPatchFormatted(input_api,
> > output_api))
> > You might want this only on upload though? I'm not sure. At least I've
> seen it
> > that way in other places, usually I don't see CheckChangeOnCommit
> anymore at
> > all.. CQ life?
>
> Hm... doesn't that mean a directly committed patch would skip the
> formatting
> check? (assuming --bypass-hooks wasn't specified)
>

Ya, I'm not sure if the CQ would block landing when you bypassed them for
whatever reason tho? Maybe it wouldn't.


>
> https://codereview.chromium.org/2489153003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mark Mentovai, 2016-11-11 00:59:05]

I’m OK with this change too, for the record.

[dcheng, 2016-11-11 18:20:25]

On 2016/11/10 23:56:02, danakj wrote:
> On Thu, Nov 10, 2016 at 3:49 PM, <mailto:dcheng@chromium.org> wrote:
> 
> > On 2016/11/10 23:06:51, danakj wrote:
> > > https://codereview.chromium.org/2489153003/diff/10001/base/PRESUBMIT.py
> > > File base/PRESUBMIT.py (right):
> > >
> > >
> > https://codereview.chromium.org/2489153003/diff/10001/
> > base/PRESUBMIT.py#newcode39
> > > base/PRESUBMIT.py:39: input_api.canned_checks.
> > CheckPatchFormatted(input_api,
> > > output_api))
> > > You might want this only on upload though? I'm not sure. At least I've
> > seen it
> > > that way in other places, usually I don't see CheckChangeOnCommit
> > anymore at
> > > all.. CQ life?
> >
> > Hm... doesn't that mean a directly committed patch would skip the
> > formatting
> > check? (assuming --bypass-hooks wasn't specified)
> >
> 
> Ya, I'm not sure if the CQ would block landing when you bypassed them for
> whatever reason tho? Maybe it wouldn't.

I think I was thinking this would help if people use git cl land directly,
rather than using the CQ at all.
On the flip side, people using git cl land are probably using --bypass-hooks...
but they might not be.
(I think it doesn't hurt too much overall, so let's just keep it?)

> 
> 
> >
> > https://codereview.chromium.org/2489153003/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-11-11 18:58:35]

On Fri, Nov 11, 2016 at 10:20 AM, <dcheng@chromium.org> wrote:

> On 2016/11/10 23:56:02, danakj wrote:
> > On Thu, Nov 10, 2016 at 3:49 PM, <mailto:dcheng@chromium.org> wrote:
> >
> > > On 2016/11/10 23:06:51, danakj wrote:
> > > > https://codereview.chromium.org/2489153003/diff/10001/
> base/PRESUBMIT.py
> > > > File base/PRESUBMIT.py (right):
> > > >
> > > >
> > > https://codereview.chromium.org/2489153003/diff/10001/
> > > base/PRESUBMIT.py#newcode39
> > > > base/PRESUBMIT.py:39: input_api.canned_checks.
> > > CheckPatchFormatted(input_api,
> > > > output_api))
> > > > You might want this only on upload though? I'm not sure. At least
> I've
> > > seen it
> > > > that way in other places, usually I don't see CheckChangeOnCommit
> > > anymore at
> > > > all.. CQ life?
> > >
> > > Hm... doesn't that mean a directly committed patch would skip the
> > > formatting
> > > check? (assuming --bypass-hooks wasn't specified)
> > >
> >
> > Ya, I'm not sure if the CQ would block landing when you bypassed them for
> > whatever reason tho? Maybe it wouldn't.
>
> I think I was thinking this would help if people use git cl land directly,
> rather than using the CQ at all.
> On the flip side, people using git cl land are probably using
> --bypass-hooks...
> but they might not be.
> (I think it doesn't hurt too much overall, so let's just keep it?)
>

Ya I am not trying to block this I'm just thinking aloud that IDK if this
would block people using the CQ if they bypassed it on upload which they
might wanna do for "reasons". I'm fine with trying it this way if u like.


>
> >
> >
> > >
> > > https://codereview.chromium.org/2489153003/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> https://codereview.chromium.org/2489153003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dcheng, 2016-11-12 03:36:26]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-11-12 03:36:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-12 04:08:19]

Description was changed from

==========
Enforce clang-format in //base

BUG=none
==========

to

==========
Enforce clang-format in //base

BUG=none
==========

[commit-bot: I haz the power, 2016-11-12 04:08:20]

Committed patchset #2 (id:10001)

[commit-bot: I haz the power, 2016-11-12 04:15:29]

Description was changed from

==========
Enforce clang-format in //base

BUG=none
==========

to

==========
Enforce clang-format in //base

BUG=none

Committed: https://crrev.com/c2f1eb57956525246abe3800fad1bfd672d99d14
Cr-Commit-Position: refs/heads/master@{#431766}
==========

[commit-bot: I haz the power, 2016-11-12 04:15:30]

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