Return the commits that make up the counts for no_review_url, tbr_no_lgtm, and blank_tbr

Return the commits that make up the counts for no_review_url, tbr_no_lgtm, and blank_tbr


BUG=514347
Committed: https://chromium.googlesource.com/infra/infra/+/679a7e3ae199b6a2d298c52bf0931b4ec06b45b1

[keelerh, 2015-07-27 20:35:52]

keelerh@google.com changed reviewers:
+ hinoka@chromium.org, pgervais@chromium.org

[pgervais, 2015-07-27 20:50:47]

Some comments. We need tests for the functions you modified. Here's how to do
it: just pass a fake cursor function as 'cc', that just ignores the call to
execute() and returns a bunch of lines on fetchall(). That would help catch
subtle errors in the subsequent processing.

https://codereview.chromium.org/1257113004/diff/1/infra/tools/antibody/comput...
File infra/tools/antibody/compute_stats.py (right):

https://codereview.chromium.org/1257113004/diff/1/infra/tools/antibody/comput...
infra/tools/antibody/compute_stats.py:344: results.append(parsed_commit)
Not sure this does what you want. parsed_commit is never reset: values will
accumulate.

Why not something like that instead of the loop?

results = [(commit[0], commit[1].strftime("%Y-%m-%d %H:%M:%S"),
            commit[2], commit[3]) for commit in result]

(and same comment for lines 373 and 403. You can even create a small helper
function)

[pgervais, 2015-07-27 23:14:07]

lgtm, with one comment

https://codereview.chromium.org/1257113004/diff/20001/infra/tools/antibody/te...
File infra/tools/antibody/test/compute_stats_test.py (right):

https://codereview.chromium.org/1257113004/diff/20001/infra/tools/antibody/te...
infra/tools/antibody/test/compute_stats_test.py:74: def
test_totaled_tbr_no_lgtm(self):
This method and the following two are basically the same test since
sql_time_specification is ignored by the mock. I think you can drop two of them.

[keelerh, 2015-07-28 00:39:32]

The CQ bit was checked by keelerh@google.com

[keelerh, 2015-07-28 00:39:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
pgervais@chromium.org
Link to the patchset: https://codereview.chromium.org/1257113004/#ps60001
(title: "Removed extraneous tests")

[commit-bot: I haz the power, 2015-07-28 00:39:39]

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

[commit-bot: I haz the power, 2015-07-28 00:42:14]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/infra/infra/+/679a7e3ae199b6a2d298c52bf0931...