[tools/perf] Delete unused variable

[tools/perf] Delete unused variable

BUG=475714
Committed: https://crrev.com/b47441d8267c3e088f1a7dc83adfa8c2115f7d7e
Cr-Commit-Position: refs/heads/master@{#398630}

[nednguyen, 2016-04-27 04:26:19]

Description was changed from

==========
[tools/perf] Delete unused variable

BUG=475714
==========

to

==========
[tools/perf] Delete unused variable

BUG=475714
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[nednguyen, 2016-04-27 04:27:05]

nednguyen@google.com changed reviewers:
+ aiolos@chromium.org, dtu@chromium.org, eakuefner@chromium.org

[eakuefner, 2016-04-27 12:17:52]

Can you please use _ in cases where there's one unused variable, or unused_* in
cases where there are 2+, per style guide?

I think the place that we landed last time we had a discussion about this was
that we should follow the style guide, and our pylintrc was updated to reflect
this. According to the style guide, referencing the variable up front (e.g.
using del) is okay to use if it's "infeasible" to rename the var, in cases where
you use keyword args and things like this.

[nednguyen, 2016-04-27 13:23:38]

On 2016/04/27 12:17:52, eakuefner (slow) wrote:
> Can you please use _ in cases where there's one unused variable, or unused_*
in
> cases where there are 2+, per style guide?

We don't always follow the Google python style guide, and the conclusion from
the last discussion is that we allow all these different styles regarding unused
variables.  As stated, I prefer program not crashing, hence will keep continue
using del.

> 
> I think the place that we landed last time we had a discussion about this was
> that we should follow the style guide, and our pylintrc was updated to reflect
> this. According to the style guide, referencing the variable up front (e.g.
> using del) is okay to use if it's "infeasible" to rename the var, in cases
where

In a big code base like telemetry, I don't think it's practical to check all the
callsites to know whether it's feasible to rename the var, and future function
call can always break it.

> you use keyword args and things like this.

[dtu, 2016-04-27 13:51:02]

On 2016/04/27 at 13:23:38, nednguyen wrote:
> On 2016/04/27 12:17:52, eakuefner (slow) wrote:
> > Can you please use _ in cases where there's one unused variable, or unused_*
in
> > cases where there are 2+, per style guide?
> 
> We don't always follow the Google python style guide, and the conclusion from
the last discussion is that we allow all these different styles regarding unused
variables.  As stated, I prefer program not crashing, hence will keep continue
using del.

We do always follow the Google Python style guide. We have had some discussions
about situations where we might want to diverge, but we haven't declared any
yet. It would, I think, require a rather unusual circumstance for us to do so.
It's probably best to discuss it on the style guide mailing list.


> > I think the place that we landed last time we had a discussion about this
was
> > that we should follow the style guide, and our pylintrc was updated to
reflect
> > this. According to the style guide, referencing the variable up front (e.g.
> > using del) is okay to use if it's "infeasible" to rename the var, in cases
where
> 
> In a big code base like telemetry, I don't think it's practical to check all
the callsites to know whether it's feasible to rename the var, and future
function call can always break it.
> 
> > you use keyword args and things like this.

[nednguyen(REVIEW IN OTHER ACC), 2016-04-27 14:04:19]

On 2016/04/27 13:51:02, dtu wrote:
> On 2016/04/27 at 13:23:38, nednguyen wrote:
> > On 2016/04/27 12:17:52, eakuefner (slow) wrote:
> > > Can you please use _ in cases where there's one unused variable, or
unused_*
> in
> > > cases where there are 2+, per style guide?
> > 
> > We don't always follow the Google python style guide, and the conclusion
from
> the last discussion is that we allow all these different styles regarding
unused
> variables.  As stated, I prefer program not crashing, hence will keep continue
> using del.
> 
> We do always follow the Google Python style guide. We have had some
discussions
> about situations where we might want to diverge, but we haven't declared any
> yet. It would, I think, require a rather unusual circumstance for us to do so.
> It's probably best to discuss it on the style guide mailing list.

I am more than happy to discuss this. Which style guide mailing list are you
talking about?
> 
> 
> > > I think the place that we landed last time we had a discussion about this
> was
> > > that we should follow the style guide, and our pylintrc was updated to
> reflect
> > > this. According to the style guide, referencing the variable up front
(e.g.
> > > using del) is okay to use if it's "infeasible" to rename the var, in cases
> where
> > 
> > In a big code base like telemetry, I don't think it's practical to check all
> the callsites to know whether it's feasible to rename the var, and future
> function call can always break it.
> > 
> > > you use keyword args and things like this.

[aiolos (Not reviewing), 2016-04-27 14:14:49]

On 2016/04/27 14:04:19, nednguyen(REVIEW IN OTHER ACC) wrote:
> On 2016/04/27 13:51:02, dtu wrote:
> > On 2016/04/27 at 13:23:38, nednguyen wrote:
> > > On 2016/04/27 12:17:52, eakuefner (slow) wrote:
> > > > Can you please use _ in cases where there's one unused variable, or
> unused_*
> > in
> > > > cases where there are 2+, per style guide?
> > > 
> > > We don't always follow the Google python style guide, and the conclusion
> from
> > the last discussion is that we allow all these different styles regarding
> unused
> > variables.  As stated, I prefer program not crashing, hence will keep
continue
> > using del.
> > 
> > We do always follow the Google Python style guide. We have had some
> discussions
> > about situations where we might want to diverge, but we haven't declared any
> > yet. It would, I think, require a rather unusual circumstance for us to do
so.
> > It's probably best to discuss it on the style guide mailing list.
> 
> I am more than happy to discuss this. Which style guide mailing list are you
> talking about?
> > 
> > 

I'll send it to you offline. Note that I'm also happy to talk about this, but
this topic isn't high enough priority to pull us away from the hackathon this
week. I'm going to stay out of the conversation until after tomorrow.

[nednguyen, 2016-06-08 03:42:16]

The "del" style is official the recommended way in python style guide. Can we
proceed with this?

[eakuefner, 2016-06-08 17:29:25]

lgtm

[nednguyen, 2016-06-08 17:41:34]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2016-06-08 17:42:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1914233003/1

[dtu, 2016-06-08 17:47:06]

lgtm

[commit-bot: I haz the power, 2016-06-08 18:43:37]

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

[commit-bot: I haz the power, 2016-06-08 18:43:38]

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

[nednguyen, 2016-06-08 18:51:13]

The CQ bit was checked by nednguyen@google.com

[nednguyen, 2016-06-08 18:51:25]

Description was changed from

==========
[tools/perf] Delete unused variable

BUG=475714
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
[tools/perf] Delete unused variable

BUG=475714
==========

[nednguyen, 2016-06-08 18:51:32]

The CQ bit was unchecked by nednguyen@google.com

[nednguyen, 2016-06-08 18:51:32]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2016-06-08 18:51:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1914233003/1

[commit-bot: I haz the power, 2016-06-08 19:06:25]

Description was changed from

==========
[tools/perf] Delete unused variable

BUG=475714
==========

to

==========
[tools/perf] Delete unused variable

BUG=475714
==========

[commit-bot: I haz the power, 2016-06-08 19:06:26]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-08 19:09:04]

Description was changed from

==========
[tools/perf] Delete unused variable

BUG=475714
==========

to

==========
[tools/perf] Delete unused variable

BUG=475714

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

[commit-bot: I haz the power, 2016-06-08 19:09:05]

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

[shatch, 2016-06-09 00:45:29]

On 2016/06/08 19:09:05, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/b47441d8267c3e088f1a7dc83adfa8c2115f7d7e
> Cr-Commit-Position: refs/heads/master@{#398630}

Seeing redness on waterfall from this, shouldn't this have gotten
CQ_EXTRA_TRYBOTS?

[nednguyen, 2016-06-09 00:47:08]

On 2016/06/09 00:45:29, shatch wrote:
> On 2016/06/08 19:09:05, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/b47441d8267c3e088f1a7dc83adfa8c2115f7d7e
> > Cr-Commit-Position: refs/heads/master@{#398630}
> 
> Seeing redness on waterfall from this, shouldn't this have gotten
> CQ_EXTRA_TRYBOTS?

I removed CQ_EXTRA_TRYBOTS because this touched like a zillion benchmarks :P

[shatch, 2016-06-09 00:47:45]

On 2016/06/09 00:47:08, nednguyen wrote:
> On 2016/06/09 00:45:29, shatch wrote:
> > On 2016/06/08 19:09:05, commit-bot: I haz the power wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/b47441d8267c3e088f1a7dc83adfa8c2115f7d7e
> > > Cr-Commit-Position: refs/heads/master@{#398630}
> > 
> > Seeing redness on waterfall from this, shouldn't this have gotten
> > CQ_EXTRA_TRYBOTS?
> 
> I removed CQ_EXTRA_TRYBOTS because this touched like a zillion benchmarks :P

Boooo :p