Update results_feed.proto, removing fields and correcting name
The field called "gerrit_change" was actually written to the database
with the name "review" in TryResult objects, and was unused in other
objects. Renamed to "review".
The "review_path" field was redundant to the review and patchset
fields, so it is removed and its uses replaced by them.
The "trivial_blamelist" field was not used for anything.
"review" field added to a Commit object, to link a commit that lands a
CL back to the Gerrit review of that CL.
Change-Id: Ic7811a5f244a99e6d52772b408187f31b8042c42
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/126881
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/functions/node/firestore_impl.dart b/functions/node/firestore_impl.dart
index 2f8b457..53806e5 100644
--- a/functions/node/firestore_impl.dart
+++ b/functions/node/firestore_impl.dart
@@ -113,7 +113,6 @@
.firstWhere(blamelistIncludesChange, orElse: () => null);
if (group == null) {
- info("Adding group for $name");
return firestore.collection('results').add(DocumentData.fromMap({
'name': name,
'result': result,
@@ -121,7 +120,6 @@
'expected': change['expected'],
'blamelist_start_index': startIndex,
'blamelist_end_index': endIndex,
- 'trivial_blamelist': startIndex == endIndex,
'configurations': <String>[change['configuration']]
}));
}
@@ -134,17 +132,9 @@
final data = groupSnapshot.data;
final newStart = max(startIndex, data.getInt('blamelist_start_index'));
final newEnd = min(endIndex, data.getInt('blamelist_end_index'));
- final updateMap = <String, dynamic>{
- 'blamelist_start_index':
- max(startIndex, data.getInt('blamelist_start_index')),
- 'blamelist_end_index': min(endIndex, data.getInt('blamelist_end_index'))
- };
- updateMap['trivial_blamelist'] = (updateMap['blamelist_start_index'] ==
- updateMap['blamelist_end_index']);
final update = UpdateData.fromMap({
'blamelist_start_index': newStart,
'blamelist_end_index': newEnd,
- 'trivial_blamelist': newStart == newEnd
});
update.setFieldValue('configurations',
Firestore.fieldValues.arrayUnion([change['configuration']]));
@@ -163,7 +153,8 @@
String previousResult = change['previous_result'] ?? 'new test';
QuerySnapshot snapshot = await firestore
.collection('try_results')
- .where('review_path', isEqualTo: reviewPath)
+ .where('review', isEqualTo: review)
+ .where('patchset', isEqualTo: patchset)
.where('name', isEqualTo: name)
.where('result', isEqualTo: result)
.where('previous_result', isEqualTo: previousResult)
@@ -172,14 +163,11 @@
.get();
if (snapshot.isEmpty) {
- info("Adding group for $name");
-
return firestore.collection('try_results').add(DocumentData.fromMap({
'name': name,
'result': result,
'previous_result': previousResult,
'expected': expected,
- 'review_path': reviewPath,
'review': review,
'patchset': patchset,
'configurations': <String>[change['configuration']]
diff --git a/functions/node/test/tools/extract_recent.dart b/functions/node/test/tools/extract_recent.dart
index 310c124..90dffdf 100644
--- a/functions/node/test/tools/extract_recent.dart
+++ b/functions/node/test/tools/extract_recent.dart
@@ -113,7 +113,7 @@
for (final review in reviews.keys) {
QuerySnapshot snapshot = await firestore
.collection('try_results')
- .where('gerritChange', isEqualTo: int.parse(review.split('/').last))
+ .where('review', isEqualTo: int.parse(review.split('/').last))
.get();
result.addAll(documentsToMap(snapshot.documents));
}
diff --git a/results_feed/doc/results_feed.proto b/results_feed/doc/results_feed.proto
index 12653c0..fd30aac 100644
--- a/results_feed/doc/results_feed.proto
+++ b/results_feed/doc/results_feed.proto
@@ -37,7 +37,6 @@
// Commits to sdk/master are indexed consecutively in the table 'commits'.
int32 blamelist_start_index = 7;
int32 blamelist_end_index = 8; // Inclusive: this commit is in the blamelist.
- bool trivial_blamelist = 9; // True if blamelist contains only one commit. Not currently used.
// Optional: The index of a the commit that is responsible for these changes.
// Chosen by a user in the results feed UI.
@@ -72,9 +71,8 @@
// Used in Result. We may want to merge the two types later.
reserved 7 to 10;
- // The Gerrit CL number. Field is currently named gerritChange in Firestore.
- // Change it to gerrit_change.
- int32 gerrit_change = 11;
+ // The Gerrit CL number.
+ int32 review = 11;
int32 patchset = 12;
// If the try result has been approved, or subsequently unapproved, by a
@@ -135,7 +133,7 @@
repeated Patchset patchsets
}
-message comment {
+message Comment {
// Unique document ID created by Firestore for this document.
string id = 1;
@@ -174,7 +172,6 @@
int32 blamelist_end_index = 11;
int32 pinned_index = 12;
// These are the fields from a TryResult message
- int32 gerrit_change = 13;
+ int32 review = 13;
int32 patchset = 14;
-
}
diff --git a/results_feed/lib/src/model/comment.dart b/results_feed/lib/src/model/comment.dart
index 95f5499..40434b5 100644
--- a/results_feed/lib/src/model/comment.dart
+++ b/results_feed/lib/src/model/comment.dart
@@ -17,7 +17,7 @@
final int blamelistStartIndex;
final int blamelistEndIndex;
final int pinnedIndex;
- final int gerritChange;
+ final int review;
final int patchset;
Comment(
@@ -33,7 +33,7 @@
this.blamelistStartIndex,
this.blamelistEndIndex,
this.pinnedIndex,
- this.gerritChange,
+ this.review,
this.patchset)
: results = List<String>.from(_results),
tryResults = List<String>.from(_tryResults);
@@ -51,7 +51,7 @@
blamelistStartIndex = document.get('blamelist_start_index'),
blamelistEndIndex = document.get('blamelist_end_index'),
pinnedIndex = document.get('pinned_index'),
- gerritChange = document.get('gerrit_change'),
+ review = document.get('review'),
patchset = document.get('patchset');
String approvedText() =>
diff --git a/results_feed/lib/src/model/commit.dart b/results_feed/lib/src/model/commit.dart
index e90a6c9..cd035dc 100644
--- a/results_feed/lib/src/model/commit.dart
+++ b/results_feed/lib/src/model/commit.dart
@@ -162,7 +162,6 @@
this.blamelistStartIndex,
this.blamelistEndIndex,
this.pinnedIndex,
- this.trivialBlamelist,
this.approved,
this.disapproved)
: changesText = '$previousResult -> $result (expected $expected)',
@@ -179,7 +178,6 @@
document.get('blamelist_start_index'),
document.get('blamelist_end_index'),
document.get('pinned_index'),
- document.get('trivial_blamelist'),
// approved field is null for unapproved new failures.
document.get('approved') == true,
document.get('approved') == false);
@@ -193,7 +191,6 @@
final int blamelistStartIndex;
final int blamelistEndIndex;
int pinnedIndex;
- bool trivialBlamelist;
bool approved;
bool disapproved; // An approval has been explicitly revoked.
final String configurationsText;
@@ -211,7 +208,6 @@
blamelistStartIndex,
blamelistEndIndex,
pinnedIndex,
- trivialBlamelist,
approved,
disapproved);
diff --git a/results_feed/lib/src/services/firestore_service.dart b/results_feed/lib/src/services/firestore_service.dart
index 1a3882d..609c7c0 100644
--- a/results_feed/lib/src/services/firestore_service.dart
+++ b/results_feed/lib/src/services/firestore_service.dart
@@ -70,7 +70,8 @@
int cl, int patch) async {
final results = app.firestore().collection('try_results');
final firestore.QuerySnapshot snapshot = await results
- .where('review_path', '==', 'refs/changes/$cl/$patch')
+ .where('review', '==', cl)
+ .where('patchset', '==', patch)
.get();
return snapshot.docs;
}
@@ -101,7 +102,7 @@
int review) async {
final results = app.firestore().collection('comments');
final firestore.QuerySnapshot snapshot =
- await results.where('gerrit_change', '==', review).get();
+ await results.where('review', '==', review).get();
return snapshot.docs;
}
@@ -150,7 +151,7 @@
if (tryResultIds != null) 'try_results': tryResultIds,
if (blamelistStart != null) 'blamelist_start_index': blamelistStart,
if (blamelistStart != null) 'blamelist_end_index': blamelistEnd,
- if (review != null) 'gerrit_change': review
+ if (review != null) 'review': review
});
if (approve == null) return;
// Update approved field in results documents.
diff --git a/results_feed/test/comments_test.dart b/results_feed/test/comments_test.dart
index 8970e5e..c9048b6 100644
--- a/results_feed/test/comments_test.dart
+++ b/results_feed/test/comments_test.dart
@@ -79,7 +79,7 @@
expect(comment.blamelistStartIndex, original['blamelist_start_index']);
expect(comment.blamelistEndIndex, original['blamelist_end_index']);
expect(comment.pinnedIndex, original['pinned_index']);
- expect(comment.gerritChange, original['gerrit_change']);
+ expect(comment.review, original['review']);
expect(comment.patchset, original['patchset']);
}
