[MD settings] specify @override on created, ready, and attached

[MD settings] specify @override on created, ready, and attached

(code health)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2847963003
Cr-Commit-Position: refs/heads/master@{#468429}
Committed: https://chromium.googlesource.com/chromium/src/+/2cbf2e6e7c4a220602ca7402bf444305a650101b

[dschuyler, 2017-04-29 01:23:41]

Description was changed from

==========
[MD settings] specify @override on created, ready, and attached

(code health)
BUG=None
==========

to

==========
[MD settings] specify @override on created, ready, and attached

(code health)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2017-04-29 01:23:43]

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

[commit-bot: I haz the power, 2017-04-29 01:24:23]

Dry run: CQ is trying da patch.

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

[dschuyler, 2017-04-29 01:24:57]

dschuyler@chromium.org changed reviewers:
+ dpapad@chromium.org

[Dan Beam, 2017-04-29 01:29:05]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2017-04-29 01:29:05]

rs lgtm but i don't really think either this nor the the 0-indent thing on //
<if> matter that much...

i'm happy to make it more consistent, though

[dpapad, 2017-04-29 01:30:29]

dpapad@chromium.org changed reviewers:
- dbeam@chromium.org

[dpapad, 2017-04-29 01:30:29]

LGTM.

One comment though (I forgot to ask/mention this in today's standup): Given that
we are still merging things to M59, perhaps it would be better to hold off
landing such horizontal changes to reduce the chance of merge conflicts.

I was going to ask in today's standup whether this is valid, or overcautious and
we don't need to wait. FWIW, I also have a fairly large change (modifying all
Licenses headers across WebUI code) and was holding off on purpose.

[dschuyler, 2017-04-29 01:32:29]

On 2017/04/29 01:29:05, Dan Beam wrote:
> rs lgtm but i don't really think either this nor the the 0-indent thing on //
> <if> matter that much...
> 
> i'm happy to make it more consistent, though

It's here because @override came up on another CL (which happens from time to
time) and I was looking to get this consistent so it was no longer one-offs in
other CLs. The formatting changes came from a format tool.
I agree it's minor.

[dpapad, 2017-04-29 01:33:41]

On 2017/04/29 at 01:29:05, dbeam wrote:
> rs lgtm but i don't really think either this nor the the 0-indent thing on //
<if> matter that much...
> 
> i'm happy to make it more consistent, though

// Intentional typo ceated instead of created
/** @override */
ceated: function() {
},

Compiler catches that error. This is the reason I ask for those annotations in
reviews. If we (collectively) don't think this is worth it, I am fine stop
asking.

[dschuyler, 2017-04-29 01:36:07]

On 2017/04/29 01:33:41, dpapad wrote:
> On 2017/04/29 at 01:29:05, dbeam wrote:
> > rs lgtm but i don't really think either this nor the the 0-indent thing on
//
> <if> matter that much...
> > 
> > i'm happy to make it more consistent, though
> 
> // Intentional typo ceated instead of created
> /** @override */
> ceated: function() {
> },
> 
> Compiler catches that error. This is the reason I ask for those annotations in
> reviews. If we (collectively) don't think this is worth it, I am fine stop
> asking.

I'm good either way. I too am happy to have it more consistent.

[dschuyler, 2017-04-29 01:36:52]

On 2017/04/29 01:30:29, dpapad wrote:
> LGTM.
> 
> One comment though (I forgot to ask/mention this in today's standup): Given
that
> we are still merging things to M59, perhaps it would be better to hold off
> landing such horizontal changes to reduce the chance of merge conflicts.

I'll hold off on landing this until after the Monday morning stand-up so that we
can talk about it then if desired.

> 
> I was going to ask in today's standup whether this is valid, or overcautious
and
> we don't need to wait. FWIW, I also have a fairly large change (modifying all
> Licenses headers across WebUI code) and was holding off on purpose.

[commit-bot: I haz the power, 2017-04-29 02:29:55]

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

[commit-bot: I haz the power, 2017-04-29 02:29:55]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2017-05-01 19:03:24]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2017-05-01 19:04:24]

CQ is trying da patch.

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

[dschuyler, 2017-05-01 19:04:55]

On 2017/04/29 01:36:52, dschuyler wrote:
> On 2017/04/29 01:30:29, dpapad wrote:
> > LGTM.
> > 
> > One comment though (I forgot to ask/mention this in today's standup): Given
> that
> > we are still merging things to M59, perhaps it would be better to hold off
> > landing such horizontal changes to reduce the chance of merge conflicts.
> 
> I'll hold off on landing this until after the Monday morning stand-up so that
we
> can talk about it then if desired.

After discussing it in the stand-up, we're going forward with code-health CLs.

> 
> > 
> > I was going to ask in today's standup whether this is valid, or overcautious
> and
> > we don't need to wait. FWIW, I also have a fairly large change (modifying
all
> > Licenses headers across WebUI code) and was holding off on purpose.

[commit-bot: I haz the power, 2017-05-01 21:46:42]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493665404427740, "parent_rev":
"982642b47574426b124f75c2e0b74f83c8705161", "commit_rev":
"2cbf2e6e7c4a220602ca7402bf444305a650101b"}

[commit-bot: I haz the power, 2017-05-01 21:46:52]

Description was changed from

==========
[MD settings] specify @override on created, ready, and attached

(code health)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] specify @override on created, ready, and attached

(code health)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2847963003
Cr-Commit-Position: refs/heads/master@{#468429}
Committed:
https://chromium.googlesource.com/chromium/src/+/2cbf2e6e7c4a220602ca7402bf44...
==========

[commit-bot: I haz the power, 2017-05-01 21:46:54]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/2cbf2e6e7c4a220602ca7402bf44...