Extract analyze-baselines command out of rebaseline.py.

Extract analyze-baselines command out of rebaseline.py.

BUG=616967
Committed: https://crrev.com/26755f9438c44e5893183a902b213901e7e9e5e3
Cr-Commit-Position: refs/heads/master@{#397586}

[qyearsley, 2016-05-29 21:07:24]

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

[commit-bot: I haz the power, 2016-05-29 21:07:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2019243002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2019243002/1

[commit-bot: I haz the power, 2016-05-29 22:09:26]

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

[commit-bot: I haz the power, 2016-05-29 22:09:27]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-06-02 17:14:54]

qyearsley@chromium.org changed reviewers:
+ dpranke@chromium.org, prasadv@chromium.org

[qyearsley, 2016-06-02 17:14:55]

There are several command classes that could be potentially extracted out of
rebaseline.py, including analyze-baselines and optimize-baselines.

Additionally, it could be reorganized so that there's one class per file, so it
might look something like:

  abstract_rebaseline_command.py
  abstract_parallel_rebaseline_command.py
  rebaseline_single_test.py
  rebaseline_multiple_tests.py
  rebaseline_from_try_jobs.py
  rebaseline_from_expectations.py
  rebaseline_from_json.py
  optimize_baselines.py
  analyze_baselines.py

Does this sound like a good idea?

[Dirk Pranke, 2016-06-02 17:56:32]

dpranke@chromium.org changed reviewers:
+ wkorman@chromium.org

[Dirk Pranke, 2016-06-02 17:56:32]

I don't feel strongly that we need a one-class-per-module approach, although I
can see the advantage to one-command-per-module.

So, I don't have strong feeling about either this patch or your suggestion to
split things up further. It kinda (but not quite) feels like busy-work to me.

Adding +wkorman for another opinion; you might check w/ others on blink-infra as
well.

lgtm if you want to go ahead, though.

[wkorman, 2016-06-02 18:10:31]

On 2016/06/02 at 17:56:32, dpranke wrote:
> I don't feel strongly that we need a one-class-per-module approach, although I
can see the advantage to one-command-per-module.
> 
> So, I don't have strong feeling about either this patch or your suggestion to
split things up further. It kinda (but not quite) feels like busy-work to me.
> 
> Adding +wkorman for another opinion; you might check w/ others on blink-infra
as well.
> 
> lgtm if you want to go ahead, though.

I actually do prefer one main class per file and considered doing something like
this. I found working in the current monolithic arrangement a challenge. Many
methods and classes are similarly named, one always has to check when reading
one to see what the containing class is, etc.

I think proceeding incrementally by trying this change where we extracti one and
work with it that way is a reasonable step to see how we like it.

[wkorman, 2016-06-02 18:10:34]

lgtm

[qyearsley, 2016-06-03 00:13:43]

Description was changed from

==========
Extract analyze-baselines command out of rebaseline.py.
==========

to

==========
Extract analyze-baselines command out of rebaseline.py.

BUG=616967
==========

[qyearsley, 2016-06-03 00:13:46]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-06-03 00:15:09]

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

[commit-bot: I haz the power, 2016-06-03 02:35:10]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-03 02:37:10]

Description was changed from

==========
Extract analyze-baselines command out of rebaseline.py.

BUG=616967
==========

to

==========
Extract analyze-baselines command out of rebaseline.py.

BUG=616967

Committed: https://crrev.com/26755f9438c44e5893183a902b213901e7e9e5e3
Cr-Commit-Position: refs/heads/master@{#397586}
==========

[commit-bot: I haz the power, 2016-06-03 02:37:11]

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