Implemented reviews stats report
* Calculate whether mark is in disagreement state (differs from later core's mark) * Add report that shows stats for last specified number of days * Don't treat 'Approve' marks as review marks * Fix table styles to show ordering arrows Implements blueprint review-disagreements Change-Id: I9590d69242b533ba1336384c43a9daa3dfc4e926
This commit is contained in:
Ilya Shakhat
parent
520fd135ec
commit
6c170a14ab
@ -157,33 +157,41 @@ def aggregate_filter():
|
||||
result[record[param_id]]['metric'] += record['loc']
|
||||
|
||||
def mark_filter(result, record, param_id):
|
||||
value = record['value']
|
||||
result_by_param = result[record[param_id]]
|
||||
result_by_param['metric'] += 1
|
||||
|
||||
if value in result_by_param:
|
||||
result_by_param[value] += 1
|
||||
if record['type'] == 'APRV':
|
||||
value = 'A'
|
||||
else:
|
||||
result_by_param[value] = 1
|
||||
value = record['value']
|
||||
result_by_param['metric'] += 1
|
||||
result_by_param[value] = result_by_param.get(value, 0) + 1
|
||||
|
||||
if record.get('x'):
|
||||
result_by_param['disagreements'] = (
|
||||
result_by_param.get('disagreements', 0) + 1)
|
||||
|
||||
def mark_finalize(record):
|
||||
new_record = {}
|
||||
for key in ['id', 'metric', 'name']:
|
||||
new_record[key] = record[key]
|
||||
new_record = record.copy()
|
||||
|
||||
positive = 0
|
||||
mark_distribution = []
|
||||
for key in [-2, -1, 1, 2]:
|
||||
for key in [-2, -1, 1, 2, 'A']:
|
||||
if key in record:
|
||||
if key in [1, 2]:
|
||||
positive += record[key]
|
||||
mark_distribution.append(str(record[key]))
|
||||
else:
|
||||
mark_distribution.append('0')
|
||||
new_record[key] = 0
|
||||
|
||||
positive_ratio = ' (%.1f%%)' % (
|
||||
(positive * 100.0) / record['metric'])
|
||||
new_record['mark_ratio'] = (
|
||||
'|'.join(mark_distribution) +
|
||||
' (%.1f%%)' % ((positive * 100.0) / record['metric']))
|
||||
'|'.join(mark_distribution) + positive_ratio)
|
||||
new_record['positive_ratio'] = positive_ratio
|
||||
new_record['disagreements'] = record.get('disagreements', 0)
|
||||
new_record['disagreement_ratio'] = '%.1f%%' % (
|
||||
(record.get('disagreements', 0) * 100.0) / record['metric']
|
||||
)
|
||||
return new_record
|
||||
|
||||
metric_param = (flask.request.args.get('metric') or
|
||||
|
||||
@ -119,7 +119,7 @@ def get_activity(records, start_record=0,
|
||||
|
||||
|
||||
def get_contribution_summary(records):
|
||||
marks = dict((m, 0) for m in [-2, -1, 0, 1, 2])
|
||||
marks = dict((m, 0) for m in [-2, -1, 0, 1, 2, 'A'])
|
||||
commit_count = 0
|
||||
loc = 0
|
||||
drafted_blueprint_count = 0
|
||||
@ -132,7 +132,10 @@ def get_contribution_summary(records):
|
||||
commit_count += 1
|
||||
loc += record['loc']
|
||||
elif record['record_type'] == 'mark':
|
||||
marks[record['value']] += 1
|
||||
if record['type'] == 'APRV':
|
||||
marks['A'] += 1
|
||||
else:
|
||||
marks[record['value']] += 1
|
||||
elif record['record_type'] == 'email':
|
||||
email_count += 1
|
||||
elif record['record_type'] == 'bpd':
|
||||
|
||||
@ -78,7 +78,7 @@ def _process_stat(data, key, time_now):
|
||||
}
|
||||
|
||||
|
||||
@blueprint.route('/reviews/<module>')
|
||||
@blueprint.route('/reviews/<module>/open')
|
||||
@decorators.templated()
|
||||
@decorators.exception_handler()
|
||||
def open_reviews(module):
|
||||
@ -115,6 +115,17 @@ def open_reviews(module):
|
||||
}
|
||||
|
||||
|
||||
@blueprint.route('/reviews/<module>/<days>')
|
||||
@decorators.templated()
|
||||
@decorators.exception_handler()
|
||||
def reviews(module, days):
|
||||
return {
|
||||
'module': module,
|
||||
'days': days,
|
||||
'start_date': int(time.time()) - int(days) * 24 * 60 * 60
|
||||
}
|
||||
|
||||
|
||||
def _get_punch_card_data(records):
|
||||
punch_card_raw = [] # matrix days x hours
|
||||
for wday in xrange(0, 7):
|
||||
|
||||
@ -16,7 +16,6 @@ border-right: 1px solid #CCC;
|
||||
background-color: #F0F3FA;
|
||||
text-align: left;
|
||||
font-weight: normal;
|
||||
background-image: none;
|
||||
border-bottom: 1px dotted #CECECE;
|
||||
text-shadow: 1px 1px 0px white;
|
||||
padding: 5px 6px;
|
||||
@ -45,10 +44,10 @@ table.dataTable td.dataTables_empty {
|
||||
table.dataTable tr.odd { background-color: #fbfafa; }
|
||||
table.dataTable tr.even { background-color: white; }
|
||||
|
||||
table.dataTable tr.odd td.sorting_1 { background-color: #eef1f4; border-right: 1px solid #CCC; }
|
||||
table.dataTable tr.odd td.sorting_1 { background-color: #eef1f4; }
|
||||
table.dataTable tr.odd td.sorting_2 { background-color: #DADCFF; }
|
||||
table.dataTable tr.odd td.sorting_3 { background-color: #E0E2FF; }
|
||||
table.dataTable tr.even td.sorting_1 { background-color: #f8f9fa; border-right: 1px solid #CCC; }
|
||||
table.dataTable tr.even td.sorting_1 { background-color: #f8f9fa; }
|
||||
table.dataTable tr.even td.sorting_2 { background-color: #F2F3FF; }
|
||||
table.dataTable tr.even td.sorting_3 { background-color: #F9F9FF; }
|
||||
|
||||
|
||||
@ -121,8 +121,10 @@ function renderTableAndChart(url, container_id, table_id, chart_id, link_param,
|
||||
}
|
||||
}
|
||||
|
||||
if (data[i].core) {
|
||||
if (data[i].core == "master") {
|
||||
data[i].link += ' ✻'
|
||||
} else if (data[i].core) {
|
||||
data[i].link += " ✬ <small><i>" + data[i].core + "</i></small>";
|
||||
}
|
||||
|
||||
tableData.push(data[i]);
|
||||
|
||||
@ -181,13 +181,15 @@
|
||||
<h2>Contribution Summary</h2>
|
||||
<div>Total commits: <b>${commit_count}</b></div>
|
||||
<div>Total LOC: <b>${loc}</b></div>
|
||||
<div>Review stat (-2, -1, +1, +2): <b>${marks["-2"]}, ${marks["-1"]}, ${marks["1"]}, ${marks["2"]}</b></div>
|
||||
<div>Review stat (-2, -1, +1, +2, A): <b>${marks["-2"]}, ${marks["-1"]}, ${marks["1"]}, ${marks["2"]}, ${marks["A"]}</b></div>
|
||||
{% endraw %}
|
||||
<div>Draft Blueprints: <b>${drafted_blueprint_count}</b></div>
|
||||
<div>Completed Blueprints: <b>${completed_blueprint_count}</b></div>
|
||||
<div>Emails: <b>${email_count}</b></div>
|
||||
{% if module %}
|
||||
<div><b><a href="/report/reviews/{{ module }}" target="_blank">Show open reviews for {{ module }}↗</a></b></div>
|
||||
<div><b><a href="/report/reviews/{{ module }}/open" target="_blank">Show open reviews for {{ module }}↗</a></b></div>
|
||||
<div><b><a href="/report/reviews/{{ module }}/30" target="_blank">Reviews for the last 30 days in {{ module }}↗</a></b></div>
|
||||
<div><b><a href="/report/reviews/{{ module }}/90" target="_blank">Reviews for the last 90 days in {{ module }}↗</a></b></div>
|
||||
{% endif %}
|
||||
{% if company %}
|
||||
<div><b><a href="/report/companies/{{ company }}" target="_blank">Show activity report for {{ company_original }}↗</a></b></div>
|
||||
@ -273,7 +275,7 @@
|
||||
<th>#</th>
|
||||
<th>Engineer</th>
|
||||
{% if show_review_ratio %}
|
||||
<th>-2|-1|+1|+2 (+/- ratio)</th>
|
||||
<th>-2|-1|+1|+2|A (+/- ratio)</th>
|
||||
{% endif %}
|
||||
<th>{{ metric_label }}</th>
|
||||
</tr>
|
||||
|
||||
126
dashboard/templates/reports/reviews.html
Normal file
126
dashboard/templates/reports/reviews.html
Normal file
@ -0,0 +1,126 @@
|
||||
{% extends "reports/base_report.html" %}
|
||||
|
||||
{% block title %}
|
||||
Reviews for the last {{ days }} days in {{ module }}
|
||||
{% endblock %}
|
||||
|
||||
{% block scripts %}
|
||||
<script type="text/javascript">
|
||||
$(document).ready(function () {
|
||||
var table_column_names = ["index", "link", "metric", "-2", "-1", "1", "2", "A", "positive_ratio", "disagreements", "disagreement_ratio"];
|
||||
var table_id = "review_stats_table";
|
||||
|
||||
$.ajax({
|
||||
url: make_uri("/api/1.0/stats/engineers?metric=marks&project_type=all&module={{ module }}&release=all&start_date={{ start_date }}"),
|
||||
dataType: "json",
|
||||
success: function (data) {
|
||||
var tableData = data["stats"];
|
||||
|
||||
var tableColumns = [];
|
||||
var sort_by_column = 0;
|
||||
for (var i = 0; i < table_column_names.length; i++) {
|
||||
tableColumns.push({"mData": table_column_names[i]});
|
||||
if (table_column_names[i] == "metric") {
|
||||
sort_by_column = i;
|
||||
}
|
||||
}
|
||||
|
||||
var summary = {
|
||||
'marks': 0,
|
||||
'core_marks': 0,
|
||||
'reviewers': tableData.length,
|
||||
'core_reviewers': 0
|
||||
};
|
||||
|
||||
for (i = 0; i < tableData.length; i++) {
|
||||
if (tableData[i].id) {
|
||||
var options = {user_id: tableData[i].id, metric: "marks"};
|
||||
var link = make_uri("/", options);
|
||||
tableData[i].link = "<a href=\"" + link + "\">" + tableData[i].name + " (" + tableData[i].id + ")</a>"
|
||||
} else {
|
||||
tableData[i].link = tableData[i].name
|
||||
}
|
||||
|
||||
if (tableData[i].core == "master") {
|
||||
tableData[i].link += " ✻";
|
||||
summary.core_marks += tableData[i].metric;
|
||||
summary.core_reviewers ++;
|
||||
} else if (tableData[i].core) {
|
||||
tableData[i].link += " ✬ <small><i>" + tableData[i].core + "</i></small>";
|
||||
}
|
||||
summary.marks += tableData[i].metric;
|
||||
}
|
||||
|
||||
if (table_id) {
|
||||
$("#" + table_id).dataTable({
|
||||
"aaSorting": [
|
||||
[ sort_by_column, "desc" ]
|
||||
],
|
||||
"bFilter": true,
|
||||
"bInfo": false,
|
||||
"bAutoWidth": false,
|
||||
"bPaginate": false,
|
||||
"iDisplayLength": -1,
|
||||
"aaData": tableData,
|
||||
"aoColumns": tableColumns,
|
||||
"aoColumnDefs": [
|
||||
{ "sClass": "center", "aTargets": [2, 3, 4, 5, 6, 7, 8, 9, 10] }
|
||||
]
|
||||
});
|
||||
}
|
||||
$("#review_summary_template").tmpl(summary).appendTo("#review_summary_container");
|
||||
}
|
||||
});
|
||||
});
|
||||
</script>
|
||||
|
||||
<style type="text/css">
|
||||
table.dataTable tr.even {
|
||||
background-color: #EEF1F4;
|
||||
}
|
||||
|
||||
table.dataTable tr.even:hover, table.dataTable tr.odd:hover {
|
||||
background-color: #F8FFEC;
|
||||
}
|
||||
|
||||
table.dataTable tr.even td.sorting_1 {
|
||||
background-color: #E0E8E8;
|
||||
}
|
||||
</style>
|
||||
|
||||
<script id="review_summary_template" type="text/x-jquery-tmpl">
|
||||
<h2>Review Summary</h2>
|
||||
<div>Total reviews: <b>${marks}</b> (${(marks / {{ days }}).toFixed(1) } per day)</div>
|
||||
<div>Total reviewers: <b>${reviewers}</b> (${(marks / reviewers / {{ days }}).toFixed(1) } per reviewer per day)</div>
|
||||
<div>Total reviews by core team: <b>${core_marks}</b> (${(core_marks / {{ days }}).toFixed(1) } per day)</div>
|
||||
<div>Core team size: <b>${core_reviewers}</b> (${(core_marks / core_reviewers / {{ days }}).toFixed(1) } per core per day)</div>
|
||||
</script>
|
||||
|
||||
{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
<h1>Reviews for the last {{ days }} days in {{ module }}</h1>
|
||||
|
||||
<table id="review_stats_table">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>#</th>
|
||||
<th>Engineer</th>
|
||||
<th>Reviews</th>
|
||||
<th>-2</th>
|
||||
<th>-1</th>
|
||||
<th>+1</th>
|
||||
<th>+2</th>
|
||||
<th>A</th>
|
||||
<th>(+/- %)</th>
|
||||
<th>Disagreements</th>
|
||||
<th>Ratio</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
</tbody>
|
||||
</table>
|
||||
|
||||
<div id="review_summary_container"></div>
|
||||
|
||||
{% endblock %}
|
||||
@ -126,11 +126,18 @@ def get_modules(records, metric_filter, finalize_handler):
|
||||
def get_engineers(records, metric_filter, finalize_handler):
|
||||
|
||||
exclude = flask.request.args.get('exclude')
|
||||
module = parameters.get_single_parameter({}, 'module')
|
||||
modules_names = parameters.get_parameter({}, 'module', 'modules')
|
||||
modules = vault.resolve_modules(modules_names)
|
||||
|
||||
def filter_core_users(record):
|
||||
user = vault.get_user_from_runtime_storage(record['id'])
|
||||
is_core = (module, 'master') in user['core']
|
||||
is_core = False
|
||||
for (module, branch) in user['core']:
|
||||
if module in modules:
|
||||
is_core = branch
|
||||
if branch == 'master': # we need master, but stables are ok
|
||||
break
|
||||
|
||||
if exclude:
|
||||
if ((exclude == 'non-core' and is_core) or
|
||||
(exclude == 'core' and not is_core)):
|
||||
@ -404,7 +411,7 @@ def timeline(records, **kwargs):
|
||||
start_date = release_start_date = _get_date(kwargs, 'start_date')
|
||||
end_date = release_end_date = _get_date(kwargs, 'end_date')
|
||||
else:
|
||||
release = releases[release_name[0]]
|
||||
release = releases[release_name]
|
||||
start_date = release_start_date = utils.timestamp_to_week(
|
||||
release['start_date'])
|
||||
end_date = release_end_date = utils.timestamp_to_week(
|
||||
@ -439,7 +446,7 @@ def timeline(records, **kwargs):
|
||||
if week in weeks:
|
||||
week_stat_loc[week] += handler(record)
|
||||
week_stat_commits[week] += 1
|
||||
if 'all' in release_name or record['release'] in release_name:
|
||||
if 'all' == release_name or record['release'] == release_name:
|
||||
week_stat_commits_hl[week] += 1
|
||||
|
||||
# form arrays in format acceptable to timeline plugin
|
||||
|
||||
@ -248,6 +248,7 @@ class RecordProcessor(object):
|
||||
|
||||
patch_sets = record.get('patchSets', [])
|
||||
review['updated_on'] = review['date']
|
||||
review['patch_count'] = len(patch_sets)
|
||||
if patch_sets:
|
||||
patch = patch_sets[-1]
|
||||
if 'approvals' in patch:
|
||||
@ -293,6 +294,7 @@ class RecordProcessor(object):
|
||||
mark['module'] = module
|
||||
mark['branch'] = branch
|
||||
mark['review_id'] = review_id
|
||||
mark['patch'] = int(patch['number'])
|
||||
|
||||
self._update_record_and_user(mark)
|
||||
|
||||
@ -532,6 +534,49 @@ class RecordProcessor(object):
|
||||
if user['core'] != core_old:
|
||||
utils.store_user(self.runtime_storage_inst, user)
|
||||
|
||||
def _marks_with_disagreement(self):
|
||||
LOG.debug('Process marks to find disagreements')
|
||||
marks_per_patch = {}
|
||||
for record in self.runtime_storage_inst.get_all_records():
|
||||
if record['record_type'] == 'mark' and record['type'] == 'CRVW':
|
||||
review_id = record['review_id']
|
||||
patch_number = record['patch']
|
||||
if (review_id, patch_number) in marks_per_patch:
|
||||
marks_per_patch[(review_id, patch_number)].append(record)
|
||||
else:
|
||||
marks_per_patch[(review_id, patch_number)] = [record]
|
||||
|
||||
cores = dict([(user['user_id'], user)
|
||||
for user in self.runtime_storage_inst.get_all_users()
|
||||
if user['core']])
|
||||
|
||||
for key, marks in marks_per_patch.iteritems():
|
||||
if len(marks) < 2:
|
||||
continue
|
||||
|
||||
core_mark = 0
|
||||
for mark in sorted(marks, key=lambda x: x['date'], reverse=True):
|
||||
|
||||
if core_mark == 0:
|
||||
user_id = mark['user_id']
|
||||
if user_id in cores:
|
||||
user = cores[user_id]
|
||||
if (mark['module'], mark['branch']) in user['core']:
|
||||
# mark is from core engineer
|
||||
core_mark = mark['value']
|
||||
continue
|
||||
|
||||
disagreement = (core_mark != 0) and (
|
||||
(core_mark < 0 < mark['value']) or
|
||||
(core_mark > 0 > mark['value']))
|
||||
old_disagreement = mark.get('x')
|
||||
mark['x'] = disagreement
|
||||
if old_disagreement != disagreement:
|
||||
yield mark
|
||||
|
||||
def finalize(self):
|
||||
self.runtime_storage_inst.set_records(
|
||||
self._get_records_for_users_to_update())
|
||||
|
||||
self.runtime_storage_inst.set_records(
|
||||
self._marks_with_disagreement())
|
||||
|
||||
@ -12,6 +12,7 @@
|
||||
# implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
import itertools
|
||||
|
||||
import mock
|
||||
import testtools
|
||||
@ -762,6 +763,61 @@ class TestRecordProcessor(testtools.TestCase):
|
||||
review2 = runtime_storage_inst.get_by_primary_key('I222')
|
||||
self.assertEquals(1, review2['review_number'])
|
||||
|
||||
def test_mark_disagreement(self):
|
||||
record_processor_inst = self.make_record_processor(
|
||||
users=[
|
||||
{'user_id': 'john_doe',
|
||||
'launchpad_id': 'john_doe',
|
||||
'user_name': 'John Doe',
|
||||
'emails': ['john_doe@ibm.com'],
|
||||
'core': [('nova', 'master')],
|
||||
'companies': [{'company_name': 'IBM', 'end_date': 0}]}
|
||||
],
|
||||
)
|
||||
runtime_storage_inst = record_processor_inst.runtime_storage_inst
|
||||
runtime_storage_inst.set_records(record_processor_inst.process([
|
||||
{'record_type': 'review',
|
||||
'id': 'I1045730e47e9e6ad31fcdfbaefdad77e2f3b2c3e',
|
||||
'subject': 'Fix AttributeError in Keypair._add_details()',
|
||||
'owner': {'name': 'John Doe',
|
||||
'email': 'john_doe@ibm.com',
|
||||
'username': 'john_doe'},
|
||||
'createdOn': 1379404951,
|
||||
'module': 'nova',
|
||||
'branch': 'master',
|
||||
'patchSets': [
|
||||
{'number': '1',
|
||||
'revision': '4d8984e92910c37b7d101c1ae8c8283a2e6f4a76',
|
||||
'ref': 'refs/changes/16/58516/1',
|
||||
'uploader': {
|
||||
'name': 'Bill Smith',
|
||||
'email': 'bill@smith.to',
|
||||
'username': 'bsmith'},
|
||||
'createdOn': 1385470730,
|
||||
'approvals': [
|
||||
{'type': 'CRVW', 'description': 'Code Review',
|
||||
'value': '1', 'grantedOn': 1385478465,
|
||||
'by': {
|
||||
'name': 'Homer Simpson',
|
||||
'email': 'hsimpson@gmail.com',
|
||||
'username': 'homer'}},
|
||||
{'type': 'CRVW', 'description': 'Code Review',
|
||||
'value': '-2', 'grantedOn': 1385478466,
|
||||
'by': {
|
||||
'name': 'John Doe',
|
||||
'email': 'john_doe@ibm.com',
|
||||
'username': 'john_doe'}}
|
||||
]
|
||||
}]}
|
||||
]))
|
||||
record_processor_inst.finalize()
|
||||
|
||||
marks = list([r for r in runtime_storage_inst.get_all_records()
|
||||
if r['record_type'] == 'mark'])
|
||||
homer_mark = next(itertools.ifilter(
|
||||
lambda x: x['date'] == 1385478465, marks), None)
|
||||
self.assertTrue(homer_mark['x']) # disagreement
|
||||
|
||||
# update records
|
||||
|
||||
def _generate_record_commit(self):
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user