[tools/perf] Enable many more lint checks for perf directory.

[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
Committed: https://crrev.com/ecdac635b48b800d43798468decf5ac5b02279dd
Cr-Commit-Position: refs/heads/master@{#411313}

[nednguyen, 2016-08-11 02:00:15]

nednguyen@google.com changed reviewers:
+ petrcermak@chromium.org

[nednguyen, 2016-08-11 02:01:01]

Description was changed from

==========
[tools/perf] Enable many more lint check for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636646
==========

to

==========
[tools/perf] Enable many more lint check for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 63401
==========

[nednguyen, 2016-08-11 02:01:19]

Description was changed from

==========
[tools/perf] Enable many more lint check for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 63401
==========

to

==========
[tools/perf] Enable many more lint check for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

[nednguyen, 2016-08-11 02:01:24]

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

[nednguyen, 2016-08-11 02:02:35]

On 2016/08/11 02:00:16, nednguyen wrote:

I verified that this catches the problem in issue 636401

[commit-bot: I haz the power, 2016-08-11 02:02:57]

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-08-11 02:09:53]

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

[commit-bot: I haz the power, 2016-08-11 02:09:54]

Dry run: 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...)

[petrcermak, 2016-08-11 08:51:13]

LGTM with a couple of nits. Thanks a lot for looking into this!

Petr

https://codereview.chromium.org/2228413003/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/loading_stories.py (right):

https://codereview.chromium.org/2228413003/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/loading_stories.py:321: 
nit: Too many blank lines here (should be only 2)

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc
File tools/perf/pylintrc (right):

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode5
tools/perf/pylintrc:5: disable=
I went through the list and it seems that while most items were removed, the
following were ADDED:

design
similarities
locally-disabled
protected-access

Just wanted to check that this is intended ;-)

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode5
tools/perf/pylintrc:5: disable=
nit: Sort the items alphabetically? It's mostly there with a few exceptions.

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode8
tools/perf/pylintrc:8: 
why the blank line?

[petrcermak, 2016-08-11 08:54:51]

description nit: s/check/checks/ (title + first line of description)

Thanks!
Petr

[nednguyen, 2016-08-11 12:23:21]

Description was changed from

==========
[tools/perf] Enable many more lint check for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

to

==========
[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

[nednguyen, 2016-08-11 12:30:30]

https://codereview.chromium.org/2228413003/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/loading_stories.py (right):

https://codereview.chromium.org/2228413003/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/loading_stories.py:321: 
On 2016/08/11 08:51:13, petrcermak wrote:
> nit: Too many blank lines here (should be only 2)

Done.

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc
File tools/perf/pylintrc (right):

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode5
tools/perf/pylintrc:5: disable=
On 2016/08/11 08:51:13, petrcermak wrote:
> nit: Sort the items alphabetically? It's mostly there with a few exceptions.

Done.

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode5
tools/perf/pylintrc:5: disable=
On 2016/08/11 08:51:13, petrcermak wrote:
> I went through the list and it seems that while most items were removed, the
> following were ADDED:
> 
> design
> similarities
> locally-disabled
> protected-access
> 
> Just wanted to check that this is intended ;-)

Done.

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode8
tools/perf/pylintrc:8: 
On 2016/08/11 08:51:13, petrcermak wrote:
> why the blank line?

Done.

[nednguyen, 2016-08-11 12:30:36]

The CQ bit was checked by nednguyen@google.com

[nednguyen, 2016-08-11 12:30:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/2228413003/#ps20001
(title: "update")

[commit-bot: I haz the power, 2016-08-11 12:30:47]

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

[nednguyen, 2016-08-11 12:32:07]

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc
File tools/perf/pylintrc (right):

https://codereview.chromium.org/2228413003/diff/1/tools/perf/pylintrc#newcode5
tools/perf/pylintrc:5: disable=
On 2016/08/11 08:51:13, petrcermak wrote:
> I went through the list and it seems that while most items were removed, the
> following were ADDED:
> 
> design
> similarities
> locally-disabled
> protected-access
> 
> Just wanted to check that this is intended ;-)

Note that locally-disabled only got triggered in this CL. I think probably due
to this presubmit check only goes over python files that are touched :-/

[commit-bot: I haz the power, 2016-08-11 13:37:26]

Description was changed from

==========
[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

to

==========
[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

[commit-bot: I haz the power, 2016-08-11 13:37:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-11 13:38:28]

Description was changed from

==========
[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401
==========

to

==========
[tools/perf] Enable many more lint checks for perf directory.

Among those that get enabled is "function-redefined" check that would prevent
people from redefining methods/classes.

BUG=475714, 636401

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

[commit-bot: I haz the power, 2016-08-11 13:38:28]

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

[Dominik Laskowski, 2016-08-15 23:44:15]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2244373003/ by domlaskowski@chromium.org.

The reason for reverting is: Breaks telemetry unit tests. See crbug.com/637938.