A few source code cosmetics for notifications.

A few source code cosmetics for notifications.

BUG=none
TBR=jianli@chromium.org (for notifications_api.cc)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285562

[Thiemo Nagel, 2014-07-24 16:35:51]

Hi Steven,

here's another tiny, cosmetic CL.  Could you please take a look?  (I'm cleanup
up old branches in my git repository...)

Thank you!
Thiemo

[stevenjb, 2014-07-24 16:44:41]

lgtm

P.S. Normally we wouldn't bother with this minor of a cosmetic change
independent of a functional or more significant re-factoring CL. On the one hand
it's not that big of a deal to review it (but still takes some time), but on the
other hand it does end up consuming try and build server resources.

[Thiemo Nagel, 2014-07-24 16:52:23]

On 2014/07/24 16:44:41, stevenjb wrote:
> lgtm

Thank you!
 
> P.S. Normally we wouldn't bother with this minor of a cosmetic change
> independent of a functional or more significant re-factoring CL. On the one
hand
> it's not that big of a deal to review it (but still takes some time), but on
the
> other hand it does end up consuming try and build server resources.

I always feel guilty when I lump functional changes together with cosmetic ones
because that makes it more difficult to understand / revert / back-merge the
functional changes.

[Thiemo Nagel, 2014-07-24 16:52:26]

The CQ bit was checked by tnagel@chromium.org

[commit-bot: I haz the power, 2014-07-24 16:53:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tnagel@chromium.org/415143002/1

[stevenjb, 2014-07-24 17:13:12]

If they are significant I would agree, but cleaning up a few includes and a
bit of WS in files that have other changes doesn't really incur any
additional overhead. Certainly less than a separate CL. Again, this is
fine, but definitely on the trivial side for a separate CL.



On Thu, Jul 24, 2014 at 9:52 AM, <tnagel@chromium.org> wrote:

> On 2014/07/24 16:44:41, stevenjb wrote:
>
>> lgtm
>>
>
> Thank you!
>
>
>  P.S. Normally we wouldn't bother with this minor of a cosmetic change
>> independent of a functional or more significant re-factoring CL. On the
>> one
>>
> hand
>
>> it's not that big of a deal to review it (but still takes some time), but
>> on
>>
> the
>
>> other hand it does end up consuming try and build server resources.
>>
>
> I always feel guilty when I lump functional changes together with cosmetic
> ones
> because that makes it more difficult to understand / revert / back-merge
> the
> functional changes.
>
> https://codereview.chromium.org/415143002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Thiemo Nagel, 2014-07-24 17:21:06]

On 2014/07/24 17:13:12, stevenjb wrote:
> If they are significant I would agree, but cleaning up a few includes and a
> bit of WS in files that have other changes doesn't really incur any
> additional overhead. Certainly less than a separate CL. Again, this is
> fine, but definitely on the trivial side for a separate CL.

Ok, thanks for your input.  I'll try to not willfully split CLs where a single
CL might be less overhead.

[commit-bot: I haz the power, 2014-07-24 20:04:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)

[commit-bot: I haz the power, 2014-07-24 20:41:11]

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

[commit-bot: I haz the power, 2014-07-24 20:41:12]

Try jobs failed on following builders:
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)

[Thiemo Nagel, 2014-07-25 09:52:46]

The CQ bit was checked by tnagel@chromium.org

[commit-bot: I haz the power, 2014-07-25 09:53:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tnagel@chromium.org/415143002/1

[commit-bot: I haz the power, 2014-07-25 10:59:36]

Change committed as 285562