[Findit] Pass changed files info to Fracas, 2 face design.

[Findit] Pass changed files info to Fracas, 2 face design.

Message format from Findit to Fracas (updated in red at the bottom of the document https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jDtE/edit#heading=h.o9pruw3u1jti)
(1)Added changed files info to results
(2)changed reason from a str to a list of tuple explaining the scores. 

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
Committed: https://chromium.googlesource.com/infra/infra/+/bf641fc0f205214cc03c192f1e1473465630507f

[Sharu Jiang, 2016-07-15 17:55:32]

Description was changed from

==========
Fix pylint


Fix pylint.


Return changed files.


Add changed files in returns.

BUG=
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

BUG=624928, 609621
==========

[Sharu Jiang, 2016-07-15 17:55:33]

katesonia@chromium.org changed reviewers:
+ chanli@chromium.org, lijeffrey@chromium.org, stgao@chromium.org

[Sharu Jiang, 2016-07-15 18:02:32]

Description was changed from

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

BUG=624928, 609621
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

[Sharu Jiang, 2016-07-15 18:04:03]

Patchset #1 (id:1) has been deleted

[Sharu Jiang, 2016-07-15 18:04:34]

PTAL :)

[Sharu Jiang, 2016-07-15 18:08:43]

On 2016/07/15 18:04:34, sharu jiang wrote:
> PTAL :)

Sorry, there are a lot of files changed, many of them are some old tests due to
the needed results fields changes. We can mainly focus on the main changes under
scorers/ dir and callstack.py and results.py :)

[stgao, 2016-07-15 23:25:28]

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/callstack.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/callstack.py:65: if not self.repo_url or not
self.dep_path:
why dep_path instead of file_path?

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/results.py:55: def __init__(self, changelog, dep_path,
Seems not needed.

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:14:
reasons_aggregator=aggregators.IdentityAggregator(),
Can we move this default value to function ``Score`` below?

[Sharu Jiang, 2016-07-18 00:31:58]

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/callstack.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/callstack.py:65: if not self.repo_url or not
self.dep_path:
On 2016/07/15 23:25:28, stgao wrote:
> why dep_path instead of file_path?

For some frames, findit cannot find a repo match, it will just set the file_path
as the original file path with out repo_path stripped.

so we always have the file_path, but only have repo_path for those we find a
repo.

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/results.py:55: def __init__(self, changelog, dep_path,
On 2016/07/15 23:25:28, stgao wrote:
> Seems not needed.

Done.

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:14:
reasons_aggregator=aggregators.IdentityAggregator(),
On 2016/07/15 23:25:28, stgao wrote:
> Can we move this default value to function ``Score`` below?

Done.

[lijeffrey, 2016-07-19 18:31:30]

lgtm with nits

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:29: # Only keep the first 2
digits after decimal point.
nit: It's not obvious the code here does the 2 digit rounding but, so take this
comment out and leave it where it's actually being rounded

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
File appengine/findit/crash/scorers/min_distance.py (right):

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
appengine/findit/crash/scorers/min_distance.py:26: for _, analysis_info in
result.file_to_analysis_info.iteritems():
since the key is not used, why not use itervalues() instead of iteritems()?

[Sharu Jiang, 2016-07-21 03:30:24]

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:29: # Only keep the first 2
digits after decimal point.
On 2016/07/19 18:31:30, lijeffrey wrote:
> nit: It's not obvious the code here does the 2 digit rounding but, so take
this
> comment out and leave it where it's actually being rounded

Done.

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
File appengine/findit/crash/scorers/min_distance.py (right):

https://codereview.chromium.org/2157433002/diff/40001/appengine/findit/crash/...
appengine/findit/crash/scorers/min_distance.py:26: for _, analysis_info in
result.file_to_analysis_info.iteritems():
On 2016/07/19 18:31:30, lijeffrey wrote:
> since the key is not used, why not use itervalues() instead of iteritems()?

Done.

[Sharu Jiang, 2016-07-21 17:50:11]

Description was changed from

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Message format from Findit to Fracas (updated in red at the bottom of the
document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

[Sharu Jiang, 2016-07-21 17:50:40]

Description was changed from

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores.

Message format from Findit to Fracas (updated in red at the bottom of the
document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores. Message format from Findit to Fracas (updated in
red at the bottom of the document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

[Sharu Jiang, 2016-07-21 17:58:59]

Description was changed from

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Add changed files info to results and changed reason from a str to a list of
tuple explaining the scores. Message format from Findit to Fracas (updated in
red at the bottom of the document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Message format from Findit to Fracas (updated in red at the bottom of the
document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)
(1)Added changed files info to results
(2)changed reason from a str to a list of tuple explaining the scores. 

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

[chanli, 2016-07-21 21:10:29]

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/parse_util.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/parse_util.py:56: A tuple - (dep_path, normalized_path),
dep_path is the dependency path
This docstring should be updated?

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/results.py:77: min_distance = float('inf')
What would be the value of float('inf')?

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:5: """Uses aggregators to
aggregator results from different scorers."""
Nit: to aggregate ?

[Sharu Jiang, 2016-07-21 21:19:33]

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/parse_util.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/parse_util.py:56: A tuple - (dep_path, normalized_path),
dep_path is the dependency path
On 2016/07/21 21:10:29, chanli wrote:
> This docstring should be updated?

Done.

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/results.py:77: min_distance = float('inf')
On 2016/07/21 21:10:29, chanli wrote:
> What would be the value of float('inf')?

It would be symbol standing for a infinite large float number.

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/scorers/aggregated_scorer.py (right):

https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/scorers/aggregated_scorer.py:5: """Uses aggregators to
aggregator results from different scorers."""
On 2016/07/21 21:10:29, chanli wrote:
> Nit: to aggregate ?

Done.

[chanli, 2016-07-21 22:35:37]

On 2016/07/21 21:19:33, sharu jiang wrote:
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> File appengine/findit/crash/parse_util.py (right):
> 
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> appengine/findit/crash/parse_util.py:56: A tuple - (dep_path,
normalized_path),
> dep_path is the dependency path
> On 2016/07/21 21:10:29, chanli wrote:
> > This docstring should be updated?
> 
> Done.
> 
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> File appengine/findit/crash/results.py (right):
> 
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> appengine/findit/crash/results.py:77: min_distance = float('inf')
> On 2016/07/21 21:10:29, chanli wrote:
> > What would be the value of float('inf')?
> 
> It would be symbol standing for a infinite large float number.
> 
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> File appengine/findit/crash/scorers/aggregated_scorer.py (right):
> 
>
https://codereview.chromium.org/2157433002/diff/80001/appengine/findit/crash/...
> appengine/findit/crash/scorers/aggregated_scorer.py:5: """Uses aggregators to
> aggregator results from different scorers."""
> On 2016/07/21 21:10:29, chanli wrote:
> > Nit: to aggregate ?
> 
> Done.

lgtm

[Sharu Jiang, 2016-07-21 22:36:59]

The CQ bit was checked by katesonia@chromium.org

[Sharu Jiang, 2016-07-21 22:37:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
lijeffrey@chromium.org
Link to the patchset: https://codereview.chromium.org/2157433002/#ps100001
(title: "Fix nits.")

[commit-bot: I haz the power, 2016-07-21 22:37:02]

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-07-21 23:14:23]

Description was changed from

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Message format from Findit to Fracas (updated in red at the bottom of the
document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)
(1)Added changed files info to results
(2)changed reason from a str to a list of tuple explaining the scores. 

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621
==========

to

==========
[Findit] Pass changed files info to Fracas, 2 face design.

Message format from Findit to Fracas (updated in red at the bottom of the
document
https://docs.google.com/document/d/1fDhJ2suyTsCLXukJwNDqtTAx_hK0hjHrOuUFFR_jD...)
(1)Added changed files info to results
(2)changed reason from a str to a list of tuple explaining the scores. 

Split previous aggregator.py into aggregators.py and aggregated_scorer.py. Add
ChangedFilesAggregator.

Fix some old tests due to the changes.

BUG=624928, 609621

Committed:
https://chromium.googlesource.com/infra/infra/+/bf641fc0f205214cc03c192f1e147...
==========

[commit-bot: I haz the power, 2016-07-21 23:14:24]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/infra/infra/+/bf641fc0f205214cc03c192f1e147...