[Findit] Display confidence score on result page.

[Findit] Display confidence score on result page.

BUG=653359
Committed: https://chromium.googlesource.com/infra/infra/+/91941e6a14c5c17ff7da1a39c5c2e7355acff51e

[chanli, 2016-10-06 01:51:52]

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

[chanli, 2016-10-06 01:51:52]

ptal.

[stgao, 2016-10-06 02:17:38]

lgtm with nits.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
File appengine/findit/handlers/build_failure.py (right):

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:20: from model.cl_confidence import
CLConfidence
Maybe SuspectConfidence instead?

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:335: confidences =
CLConfidence.query().order(-CLConfidence.end_date).fetch(1)
As I mentioned in the other CL, making CLConfidence versioned will make this
easier and faster with cache.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:343: return '%.2f%%' % (float_number
* 100)
I guess 70% is better than 70.87%.

[lijeffrey, 2016-10-06 14:10:33]

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
File appengine/findit/handlers/build_failure.py (right):

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:335: confidences =
CLConfidence.query().order(-CLConfidence.end_date).fetch(1)
On 2016/10/06 02:17:38, stgao (slow) wrote:
> As I mentioned in the other CL, making CLConfidence versioned will make this
> easier and faster with cache.

+1

if we make CLConfidence versioned then this could be done in 1 line

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:346: def _GetConfidence(confidence,
cl_build):
nit: rename this _GetConfidencePercentage or _GetConfidenceScore or something
more accurate, since GetConfidence sounds like it returns a confidence entity

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:374: else:  # pragma: no cover.
why is this one no cover whereas the compile one does have it?

[chanli, 2016-10-08 00:41:10]

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
File appengine/findit/handlers/build_failure.py (right):

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:20: from model.cl_confidence import
CLConfidence
On 2016/10/06 02:17:38, stgao (slow) wrote:
> Maybe SuspectConfidence instead?

Done.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:335: confidences =
CLConfidence.query().order(-CLConfidence.end_date).fetch(1)
On 2016/10/06 02:17:38, stgao (slow) wrote:
> As I mentioned in the other CL, making CLConfidence versioned will make this
> easier and faster with cache.

Done.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:343: return '%.2f%%' % (float_number
* 100)
On 2016/10/06 02:17:38, stgao (slow) wrote:
> I guess 70% is better than 70.87%.

Done.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:346: def _GetConfidence(confidence,
cl_build):
On 2016/10/06 14:10:33, lijeffrey wrote:
> nit: rename this _GetConfidencePercentage or _GetConfidenceScore or something
> more accurate, since GetConfidence sounds like it returns a confidence entity

Done.

https://codereview.chromium.org/2398903002/diff/1/appengine/findit/handlers/b...
appengine/findit/handlers/build_failure.py:374: else:  # pragma: no cover.
On 2016/10/06 14:10:33, lijeffrey wrote:
> why is this one no cover whereas the compile one does have it?

Done.

[chanli, 2016-10-08 00:41:54]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-10-08 00:41:55]

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

[commit-bot: I haz the power, 2016-10-08 00:42:00]

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

[chanli, 2016-10-08 00:48:46]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-10-08 00:48:47]

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

[commit-bot: I haz the power, 2016-10-08 00:48:49]

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-08 01:03:15]

Description was changed from

==========
[Findit] Display confidence score on result page.

BUG=653359
==========

to

==========
[Findit] Display confidence score on result page.

BUG=653359

Committed:
https://chromium.googlesource.com/infra/infra/+/91941e6a14c5c17ff7da1a39c5c2e...
==========

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

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/91941e6a14c5c17ff7da1a39c5c2e...