Lint suppressions needed for Android SDK roll.

Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting them up into separate CLs to make OWNERS review easier.

BUG=623989
Committed: https://crrev.com/4c3bcc5d9fee0074cb50aee243bfd75ebc33d542
Cr-Commit-Position: refs/heads/master@{#422441}

[ghost stip (do not use), 2016-09-23 21:01:27]

Description was changed from

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989
==========

to

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989
==========

[ghost stip (do not use), 2016-09-23 21:01:27]

stip@chromium.org changed reviewers:
+ jbudorick@chromium.org, tedchoc@chromium.org

[ghost stip (do not use), 2016-09-23 21:01:43]

jbudorick: ptal
tedchoc: OWNERS

[jbudorick, 2016-09-23 21:24:14]

lgtm

[ghost stip (do not use), 2016-09-27 23:59:06]

Ted, any thoughts?

[Ted C, 2016-09-28 05:52:31]

tedchoc@chromium.org changed reviewers:
+ agrieve@chromium.org, estevenson@chromium.org

[Ted C, 2016-09-28 05:52:31]

While this seems like the right thing to unblock the roll, I'm worried about
suppressing lint items that might be legitimate issues.  As far as I know, we
only use @SuppressLint for cases where it can either not be addressed or the
linter is in fact incorrectly causing errors.  In these cases, it doesn't seem
to be the case.

I wonder if we should add these but tag them in some way that is easily
searchable?

@SuppressLint("...") // TODO(someone): Required for N-SDK, Investigate root
cause.

Or something with a bit more pizzazz.

+agrieve, +estevenson for their work on lint recently

Sorry for the delay on the change by the way!  Ping me if you don't hear back in
a day (or two at the most).

[agrieve, 2016-09-28 14:05:59]

On 2016/09/28 05:52:31, Ted C wrote:
> While this seems like the right thing to unblock the roll, I'm worried about
> suppressing lint items that might be legitimate issues.  As far as I know, we
> only use @SuppressLint for cases where it can either not be addressed or the
> linter is in fact incorrectly causing errors.  In these cases, it doesn't seem
> to be the case.
> 
> I wonder if we should add these but tag them in some way that is easily
> searchable?
> 
> @SuppressLint("...") // TODO(someone): Required for N-SDK, Investigate root
> cause.
> 
> Or something with a bit more pizzazz.
> 
> +agrieve, +estevenson for their work on lint recently
> 
> Sorry for the delay on the change by the way!  Ping me if you don't hear back
in
> a day (or two at the most).

I think I'd agree with this. It would be ideal if each @SuppressLint had a
comment explaining why it's the right thing to do. 

For now, how about adding all of these to suppressions.xml, and so that the file
will act as a list of warnings to go through.

[jbudorick, 2016-09-28 14:09:40]

On 2016/09/28 14:05:59, agrieve wrote:
> On 2016/09/28 05:52:31, Ted C wrote:
> > While this seems like the right thing to unblock the roll, I'm worried about
> > suppressing lint items that might be legitimate issues.  As far as I know,
we
> > only use @SuppressLint for cases where it can either not be addressed or the
> > linter is in fact incorrectly causing errors.  In these cases, it doesn't
seem
> > to be the case.
> > 
> > I wonder if we should add these but tag them in some way that is easily
> > searchable?
> > 
> > @SuppressLint("...") // TODO(someone): Required for N-SDK, Investigate root
> > cause.
> > 
> > Or something with a bit more pizzazz.
> > 
> > +agrieve, +estevenson for their work on lint recently
> > 
> > Sorry for the delay on the change by the way!  Ping me if you don't hear
back
> in
> > a day (or two at the most).
> 
> I think I'd agree with this. It would be ideal if each @SuppressLint had a
> comment explaining why it's the right thing to do. 
> 
> For now, how about adding all of these to suppressions.xml, and so that the
file
> will act as a list of warnings to go through.

I'm strongly opposed to adding these to suppressions.xml, where I think they're
more likely to be forgotten than if they appear in-line.

[agrieve, 2016-09-28 14:16:23]

On 2016/09/28 14:09:40, jbudorick wrote:
> On 2016/09/28 14:05:59, agrieve wrote:
> > On 2016/09/28 05:52:31, Ted C wrote:
> > > While this seems like the right thing to unblock the roll, I'm worried
about
> > > suppressing lint items that might be legitimate issues.  As far as I know,
> we
> > > only use @SuppressLint for cases where it can either not be addressed or
the
> > > linter is in fact incorrectly causing errors.  In these cases, it doesn't
> seem
> > > to be the case.
> > > 
> > > I wonder if we should add these but tag them in some way that is easily
> > > searchable?
> > > 
> > > @SuppressLint("...") // TODO(someone): Required for N-SDK, Investigate
root
> > > cause.
> > > 
> > > Or something with a bit more pizzazz.
> > > 
> > > +agrieve, +estevenson for their work on lint recently
> > > 
> > > Sorry for the delay on the change by the way!  Ping me if you don't hear
> back
> > in
> > > a day (or two at the most).
> > 
> > I think I'd agree with this. It would be ideal if each @SuppressLint had a
> > comment explaining why it's the right thing to do. 
> > 
> > For now, how about adding all of these to suppressions.xml, and so that the
> file
> > will act as a list of warnings to go through.
> 
> I'm strongly opposed to adding these to suppressions.xml, where I think
they're
> more likely to be forgotten than if they appear in-line.

How about adding to all of them: // TODO(crbug.com/635567): Fix this properly.

[jbudorick, 2016-09-28 14:18:18]

On 2016/09/28 14:16:23, agrieve wrote:
> On 2016/09/28 14:09:40, jbudorick wrote:
> > On 2016/09/28 14:05:59, agrieve wrote:
> > > On 2016/09/28 05:52:31, Ted C wrote:
> > > > While this seems like the right thing to unblock the roll, I'm worried
> about
> > > > suppressing lint items that might be legitimate issues.  As far as I
know,
> > we
> > > > only use @SuppressLint for cases where it can either not be addressed or
> the
> > > > linter is in fact incorrectly causing errors.  In these cases, it
doesn't
> > seem
> > > > to be the case.
> > > > 
> > > > I wonder if we should add these but tag them in some way that is easily
> > > > searchable?
> > > > 
> > > > @SuppressLint("...") // TODO(someone): Required for N-SDK, Investigate
> root
> > > > cause.
> > > > 
> > > > Or something with a bit more pizzazz.
> > > > 
> > > > +agrieve, +estevenson for their work on lint recently
> > > > 
> > > > Sorry for the delay on the change by the way!  Ping me if you don't hear
> > back
> > > in
> > > > a day (or two at the most).
> > > 
> > > I think I'd agree with this. It would be ideal if each @SuppressLint had a
> > > comment explaining why it's the right thing to do. 
> > > 
> > > For now, how about adding all of these to suppressions.xml, and so that
the
> > file
> > > will act as a list of warnings to go through.
> > 
> > I'm strongly opposed to adding these to suppressions.xml, where I think
> they're
> > more likely to be forgotten than if they appear in-line.
> 
> How about adding to all of them: // TODO(crbug.com/635567): Fix this properly.

That'd be fine with me.

[ghost stip (do not use), 2016-09-29 00:03:01]

I've added the TODO (and will send CLs for the others that have landed to add
the same TODO). ptal!

[agrieve, 2016-09-29 00:04:44]

On 2016/09/29 00:03:01, stip wrote:
> I've added the TODO (and will send CLs for the others that have landed to add
> the same TODO). ptal!

lgtm

[Ted C, 2016-09-29 00:10:12]

lgtm

[ghost stip (do not use), 2016-09-29 00:56:48]

The CQ bit was checked by stip@chromium.org

[ghost stip (do not use), 2016-09-29 00:56:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2363113003/#ps20001
(title: "Add TODO link.")

[commit-bot: I haz the power, 2016-09-29 00:57:19]

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-09-29 01:08:07]

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

[commit-bot: I haz the power, 2016-09-29 01:08:09]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ghost stip (do not use), 2016-10-01 02:20:46]

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

[commit-bot: I haz the power, 2016-10-01 02:21:02]

Dry run: 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-10-01 03:04:03]

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

[commit-bot: I haz the power, 2016-10-01 03:04:04]

Dry run: This issue passed the CQ dry run.

[jbudorick, 2016-10-03 16:26:26]

still lgtm

[ghost stip (do not use), 2016-10-03 16:26:54]

The CQ bit was checked by stip@chromium.org

[ghost stip (do not use), 2016-10-03 16:26:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2363113003/#ps80001
(title: "Suppress alert box presubmit.")

[commit-bot: I haz the power, 2016-10-03 16:27:11]

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-10-03 17:09:22]

Description was changed from

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989
==========

to

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989
==========

[commit-bot: I haz the power, 2016-10-03 17:09:23]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-03 17:11:59]

Description was changed from

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989
==========

to

==========
Lint suppressions needed for Android SDK roll.

This is only part of the suppressions needed for a full compile, I'm splitting
them up into separate CLs to make OWNERS review easier.

BUG=623989

Committed: https://crrev.com/4c3bcc5d9fee0074cb50aee243bfd75ebc33d542
Cr-Commit-Position: refs/heads/master@{#422441}
==========

[commit-bot: I haz the power, 2016-10-03 17:12:00]

Patchset 5 (id:??) landed as
https://crrev.com/4c3bcc5d9fee0074cb50aee243bfd75ebc33d542
Cr-Commit-Position: refs/heads/master@{#422441}