[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
Committed: https://crrev.com/4408d6ca936c694ceb0574f9f59160aade65f0ea
Cr-Commit-Position: refs/heads/master@{#433532}

[vitaliii, 2016-11-21 08:42:59]

The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 08:43:11]

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

[vitaliii, 2016-11-21 08:46:04]

Description was changed from

==========
[NTP] Cleanup: add brackets in components/ntp_snippets [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp.

BUG=667202
==========

to

==========
[NTP] Cleanup: add brackets in components/ntp_snippets [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

[vitaliii, 2016-11-21 08:50:48]

vitaliii@chromium.org changed reviewers:
+ treib@chromium.org

[vitaliii, 2016-11-21 08:50:49]

Hi Marc,

could you have a look?

https://codereview.chromium.org/2520853002/diff/1/components/ntp_snippets/rem...
File components/ntp_snippets/remote/ntp_snippets_status_service.cc (left):

https://codereview.chromium.org/2520853002/diff/1/components/ntp_snippets/rem...
components/ntp_snippets/remote/ntp_snippets_status_service.cc:37: } else if
(!param_value_str.empty() &&
We have not explicitly considered this case, but my assumption is that we add
brackets here as well. Do you agree?

[vitaliii, 2016-11-21 08:56:45]

Description was changed from

==========
[NTP] Cleanup: add brackets in components/ntp_snippets [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

to

==========
[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

[Marc Treib, 2016-11-21 09:41:22]

https://codereview.chromium.org/2520853002/diff/1/components/ntp_snippets/rem...
File components/ntp_snippets/remote/ntp_snippets_status_service.cc (left):

https://codereview.chromium.org/2520853002/diff/1/components/ntp_snippets/rem...
components/ntp_snippets/remote/ntp_snippets_status_service.cc:37: } else if
(!param_value_str.empty() &&
On 2016/11/21 08:50:49, vitaliii wrote:
> We have not explicitly considered this case, but my assumption is that we add
> brackets here as well. Do you agree?

No, "else if" is a very common pattern, and I'd leave that alone. IMO adding the
extra braces doesn't improve readability, and it introduces an extra level of
indentation.

[vitaliii, 2016-11-21 09:49:06]

The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 09:49:20]

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

[vitaliii, 2016-11-21 09:49:24]

Hi Marc,

I reverted back "else if" bit, please have a look.

[Marc Treib, 2016-11-21 10:00:36]

LGTM!
And btw, thanks for the super-speedy cleanup!

[vitaliii, 2016-11-21 12:01:00]

The CQ bit was unchecked by vitaliii@chromium.org

[vitaliii, 2016-11-21 12:01:03]

The CQ bit was checked by vitaliii@chromium.org

[commit-bot: I haz the power, 2016-11-21 12:01:06]

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

[vitaliii, 2016-11-21 14:12:46]

The CQ bit was unchecked by vitaliii@chromium.org

[vitaliii, 2016-11-21 14:12:51]

The CQ bit was checked by vitaliii@chromium.org

[commit-bot: I haz the power, 2016-11-21 14:13:07]

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-21 14:16:27]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1479737571658530,
"parent_rev": ["1457ba5370a94b6f702500df03e390aea54618f9", null], "commit_rev":
["da7c30783f3de4a1ac67adc9a291900421d39259", null]}

[commit-bot: I haz the power, 2016-11-21 14:16:49]

Description was changed from

==========
[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

to

==========
[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

[commit-bot: I haz the power, 2016-11-21 14:16:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-21 14:19:00]

Description was changed from

==========
[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202
==========

to

==========
[NTP] Cleanup: add brackets in components/ntp_snippets. [2/2]

This CL adds curly brackets to |if|, |for| and |while| in
components/ntp_snippets. The search was done using
"((if)|(for)|(while)) \([^{]*?\n" regexp. Note that it matches
multiline statements and this was intended, since there were
not many such cases and I wanted to confirm that we did not
miss brackets there.

BUG=667202

Committed: https://crrev.com/4408d6ca936c694ceb0574f9f59160aade65f0ea
Cr-Commit-Position: refs/heads/master@{#433532}
==========

[commit-bot: I haz the power, 2016-11-21 14:19:01]

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