Protobuf-backed FieldValues #1184

schmidt-sebastian · 2020-02-01T01:26:46Z

This finishes up the Protobuf FieldValue rewrite:

Merges PrimitiveValue with FieldValue
Moves protovalue classes to value package
Implements ServerTimestamp support

The individual FieldValue types (IntegerValue, DoubleValue, etc) still exists, which makes the diff considerably smaller and keeps their current factory methods.

Performance with master for returning X docs out a collection of size Y:
20 out of 1000: 3692 505 382 310 524 228 409 356
1000 out of 1000: 8813 1512 1066 1748 997 1114 1145

With this branch:
20 out of 1000: 714 238 279 311 277 339 245 308
1000 out of 1000: 1344 1186 677 767 726 657 673 636

I will get more numbers (once I have fixed the Performance spec tests, but I will do that tomorrow).

codecov · 2020-02-04T04:14:33Z

Codecov Report

Merging #1184 into mrschmidt/rewritefieldvalue will increase coverage by 7.53%.
The diff coverage is 83.72%.

Flag	Coverage Δ	Complexity Δ
#Encoders_FirebaseEncodersJson	`?`	`?`
#Encoders_FirebaseEncodersProcessor	`?`	`?`
#Encoders_FirebaseEncodersReflective	`?`	`?`
#FirebaseAbt	`?`	`?`
#FirebaseCommon	`?`	`?`
#FirebaseCommon_DataCollectionTests	`?`	`?`
#FirebaseCommon_Ktx	`?`	`?`
#FirebaseComponents	`?`	`?`
#FirebaseConfig	`?`	`?`
#FirebaseConfig_Ktx	`?`	`?`
#FirebaseCrashlytics	`?`	`?`
#FirebaseDatabase	`?`	`?`
#FirebaseDatabaseCollection	`?`	`?`
#FirebaseDatabase_Ktx	`?`	`?`
#FirebaseDatatransport	`?`	`?`
#FirebaseDynamicLinks	`?`	`?`
#FirebaseDynamicLinks_Ktx	`?`	`?`
#FirebaseFirestore	`62.24% <83.72%> (-37.76%)`	`2258 <15> (+2258)`
#FirebaseFirestore_Ktx	`41.17% <ø> (-58.83%)`	`0 <ø> (ø)`
#FirebaseFunctions	`?`	`?`
#FirebaseFunctions_Ktx	`?`	`?`
#FirebaseInappmessaging	`?`	`?`
#FirebaseInappmessagingDisplay	`?`	`?`
#FirebaseInappmessagingDisplay_Ktx	`?`	`?`
#FirebaseInappmessaging_Ktx	`?`	`?`
#FirebaseInstallations	`?`	`?`
#FirebaseSegmentation	`?`	`?`
#FirebaseStorage	`?`	`?`
#FirebaseStorage_Ktx	`?`	`?`
#Tools_Errorprone	`?`	`?`
#Tools_Lint	`?`	`?`
#Transport_TransportBackendCct	`?`	`?`
#Transport_TransportRuntime	`?`	`?`

Impacted Files	Coverage Δ	Complexity Δ
...estore/model/mutation/ArrayTransformOperation.java	`79.41% <ø> (ø)`	`8 <0> (ø)`	⬇️
...gle/firebase/firestore/model/value/ArrayValue.java	`100% <ø> (ø)`	`5 <0> (-3)`	⬇️
...e/firebase/firestore/core/ArrayContainsFilter.java	`100% <100%> (ø)`	`4 <0> (ø)`	⬇️
...le/firebase/firestore/model/value/ObjectValue.java	`94.56% <100%> (-0.23%)`	`19 <0> (ø)`
...le/firebase/firestore/remote/RemoteSerializer.java	`76.48% <100%> (ø)`	`113 <0> (ø)`	⬇️
...se/firestore/model/value/ServerTimestampValue.java	`78.57% <100%> (+1.64%)`	`7 <0> (ø)`	⬇️
...se/firestore/model/mutation/TransformMutation.java	`81.92% <100%> (ø)`	`22 <0> (ø)`	⬇️
...le/firebase/firestore/model/value/ProtoValues.java	`91.66% <100%> (+0.2%)`	`94 <3> (+3)`	⬆️
...irebase/firestore/core/ArrayContainsAnyFilter.java	`100% <100%> (ø)`	`5 <0> (ø)`	⬇️
...e/firebase/firestore/local/IndexedQueryEngine.java	`56.71% <100%> (ø)`	`14 <0> (ø)`	⬇️
... and 702 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 804a1b7...98953a3. Read the comment docs.

schmidt-sebastian · 2020-02-04T04:23:10Z

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/WriteBatchTest.java

@@ -222,7 +222,7 @@ public void testWriteTheSameServerTimestampAcrossWrites() {
    assertTrue(localSnap.getMetadata().hasPendingWrites());
    assertEquals(asList(map("when", null), map("when", null)), querySnapshotToValues(localSnap));

-    QuerySnapshot serverSnap = accumulator.await();
+    QuerySnapshot serverSnap = accumulator.awaitRemoteEvent();


This PR somehow changed the timing of events so much that the fromCache:false event is now delivered before hasPendingWrites:false. I am not 100% convinced that this is not due to a bug, but if I look at the test logs everything looks sane.

schmidt-sebastian · 2020-02-04T04:23:32Z

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java

-            onlineCallbackDone.setResult(null);
-          }
-        });
+    ListenerRegistration listenerRegistration =


This is the change from #1188

schmidt-sebastian · 2020-02-04T04:25:10Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -32,16 +32,24 @@
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.GeoPoint;
 import com.google.firebase.firestore.model.mutation.FieldMask;
-import com.google.firebase.firestore.model.protovalue.ObjectValue;
-import com.google.firebase.firestore.model.protovalue.PrimitiveValue;
+import com.google.firebase.firestore.model.value.BlobValue;


FYI: A lot of the diff in this file brings the test more in line with master.

wilhuff

I've done a partial review but want to give you the feedback I have so far.

wilhuff · 2020-02-04T23:14:25Z

codecov.yml

@@ -9,12 +9,14 @@ coverage:
      default: off
      FirebaseCommon:
        flags: FirebaseCommon
-        informational: true
+        informational: false


Are these changes intended?

If so, since these are changes to FirebaseCommon's configuration, maybe pull them out into a PR sent to someone that owns that?

I merged master into rewritefieldvalue and rewritefieldvalue into this branch, but forgot to push rewritefieldvalue. This should be fixed.

wilhuff · 2020-02-04T23:18:36Z

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java

@@ -533,7 +533,7 @@ private Object getInternal(
      if (val != null) {
        UserDataWriter userDataWriter =
            new UserDataWriter(firestore, timestampsInSnapshots, serverTimestampBehavior);
-        return userDataWriter.convertValue(val);
+        return userDataWriter.convertValue(val.getProto());


It seems like having overloads (or just a public interface) that accepts FieldValue/ObjectValue would make this code less cumbersome to deal with.

Then again, if this is the only caller, maybe this is better.

Just a thought; no action required.

This pattern is used in two places in the code and in a bunch of test cases in UserWriterTest. I added a helper to UserDataWriterTest, but kept the usages in the code the same.

wilhuff · 2020-02-04T23:25:12Z

firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataReader.java

@@ -398,35 +389,37 @@ private void parseSentinelFieldValue(
   * @return The parsed value, or {@code null} if the value was a FieldValue sentinel that should
   *     not be included in the resulting parsed data.
   */
-  private FieldValue parseScalarValue(Object input, ParseContext context) {
+  private Value parseScalarValue(Object input, ParseContext context) {


This code essentially duplicates Values.valueOf. Is there no way to unite these?

I added a TODO for now. I would like to remove the individual field value wrappers first.

wilhuff · 2020-02-04T23:28:43Z

firebase-firestore/src/main/java/com/google/firebase/firestore/core/ArrayContainsAnyFilter.java

@@ -32,8 +32,8 @@ public boolean matches(Document doc) {
    if (!(other instanceof ArrayValue)) {
      return false;
    }
-    for (FieldValue val : ((ArrayValue) other).getInternalValue()) {
-      if (arrayValue.getInternalValue().contains(val)) {
+    for (FieldValue val : ((ArrayValue) other).getValues()) {


It seems like this would be cheaper if we iterated over the proto Value elements and compared for equality using ProtoValues directly.

Feel free to defer into another PR that address concerns like these.

Done. More cleanup and the removal of ArrayValue will come in a follow-up.

wilhuff · 2020-02-04T23:33:56Z

firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ArrayValue.java

+   */
+  public boolean contains(FieldValue value) {
+    for (Value element : internalValue.getArrayValue().getValuesList()) {
+      if (value.equals(FieldValue.valueOf(element))) {


If you pull the proto out of the value outside the loop you can use ProtoValues.equals here instead which avoids the reflective type checking and allocation inherent in FieldValue.valueOf.

I updated the callers of this method to do exactly that.

wilhuff · 2020-02-04T23:41:34Z

firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ObjectValue.java

 import java.util.Set;

 /** A structured object value stored in Firestore. */
 public class ObjectValue extends FieldValue {
  private static final ObjectValue EMPTY_INSTANCE =
-      new ObjectValue(ImmutableSortedMap.Builder.emptyMap(Util.<String>comparator()));
+      new ObjectValue(
+          com.google.firestore.v1.Value.newBuilder()


Shorten this to use the imported Value and MapValue? Here and below.

wilhuff · 2020-02-04T23:47:21Z

firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java

@@ -165,7 +166,8 @@ public void setLocalDocumentsView(LocalDocumentsView localDocuments) {
  private static double estimateFilterSelectivity(Filter filter) {
    hardAssert(filter instanceof FieldFilter, "Filter type expected to be FieldFilter");
    FieldFilter fieldFilter = (FieldFilter) filter;
-    if (fieldFilter.getValue().equals(null) || fieldFilter.getValue().equals(DoubleValue.NaN)) {
+    FieldValue filterValue = fieldFilter.getValue();
+    if (NullValue.nullValue().equals(filterValue) || DoubleValue.NaN.equals(filterValue)) {


It strikes me here that we're inconsistent about how we handle these singletons. We could have NullValue.NULL instead, right?

wilhuff · 2020-02-04T23:50:01Z

...-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java

        }
+        baseObject = baseObject.set(transform.getFieldPath(), coercedValue.getProto());


Now that baseObject is a Builder, this assignment here seems duplicative. The builder only does return this for chaining, right?

Yes, this is not needed anymore. We also don't use chaining anywhere - I wonder if we should remove it from the Builder API.

wilhuff · 2020-02-04T23:58:44Z

firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java

-    } else {
-      return new PrimitiveValue(value);
+  public static @Nullable FieldValue valueOf(@Nullable Value value) {
+    if (value != null) {


Use an early return to reduce nesting?

wilhuff · 2020-02-05T00:01:01Z

...-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java

-    this.localWriteTime = localWriteTime;
-    this.previousValue = previousValue;
+  public ServerTimestampValue(Value value) {
+    super(value);


hardAssert that isServerTimestamp(value)?

Done. Also made the constructor package-private.

wilhuff

A few more thoughts.

wilhuff · 2020-02-05T16:50:24Z

firebase-firestore/src/test/java/com/google/firebase/firestore/UserDataWriterTest.java

@@ -167,8 +179,8 @@ public void testConvertsTimestampValue() {
    for (Timestamp d : testCases) {


It seems like the other tests are using a single letter variable that's short for the type. Make this Timestamp t?

wilhuff · 2020-02-05T16:51:43Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

-                field("foo.bar"),
-                new ServerTimestampValue(timestamp, StringValue.valueOf("bar-value")))
-            .build();
+    com.google.firebase.firestore.model.value.FieldValue fieldValue =


Import FieldValue and shorten?

This test already imports com.google.firebase.firestore.FieldValue.

wilhuff · 2020-02-05T17:00:51Z

...base-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java

@@ -74,7 +74,7 @@ public void setsNestedField() {
    builder.set(field("a.b"), fooValue);
    builder.set(field("c.d.e"), fooValue);
    ObjectValue object = builder.build();
-    assertEquals(wrapObject("a", map("b", fooValue), "c", map("d", map("e", fooValue))), object);
+    assertEquals(wrapObject("a", map("b", fooString), "c", map("d", map("e", fooString))), object);


fooString isn't adding a whole lot of value here, and it's inconsistent with the other string literals here. Consider reverting that.

Also, why not pass the values as arguments to map? That seemed like a great way to explicitly compose expected values.

wilhuff · 2020-02-05T17:03:09Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -118,8 +127,13 @@ public void testAddsNewFields() {
  @Test
  public void testAddsMultipleNewFields() {
    ObjectValue object = ObjectValue.emptyObject();
-    object = object.toBuilder().set(field("a"), valueOf("a")).build();
-    object = object.toBuilder().set(field("b"), valueOf("b")).set(field("c"), valueOf("c")).build();
+    object = object.toBuilder().set(field("a"), wrap("a").getProto()).build();


FWIW, valueOf("a") seems cleaner than wrap("a").getProto().

Replaced this single occurrence with a call to the helper setField.

wilhuff · 2020-02-05T17:04:23Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -137,7 +151,7 @@ public void testImplicitlyCreatesObjects() {
  @Test
  public void testCanOverwritePrimitivesWithObjects() {
    ObjectValue old = wrapObject("a", map("b", "old"));
-    ObjectValue mod = setField(old, "a", map("b", "mod"));
+    ObjectValue mod = setField(old, "a", wrapObject("b", "mod"));


We should add an overload for setField that allows map to work inline so that we don't have think about this distinction.

wilhuff · 2020-02-05T17:18:37Z

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

-        com.google.firestore.v1.ArrayValue.newBuilder();
-    for (FieldValue subValue : elements) {
-      arrayBuilder.addValues(encodeValue(subValue));
+  private ArrayValue encodeArrayTransformElements(List<FieldValue> elementsProto) {


List<FieldValue> still seems like "elements" more than "elementsProto": we're asking each element to get its underlying proto.

wilhuff · 2020-02-05T17:20:15Z

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

-    for (int i = 0; i < count; i++) {
-      result.add(decodeValue(elementsProto.getValues(i)));
+  private List<FieldValue> decodeArrayTransformElements(List<Value> elementsProto) {
+    List<FieldValue> result = new ArrayList<>(elementsProto.size());


Is it possible to make the array transform elements just a model.ArrayValue?

wilhuff · 2020-02-05T17:21:11Z

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -648,7 +482,7 @@ public MutationResult decodeMutationResult(
    if (transformResultsCount > 0) {
      transformResults = new ArrayList<>(transformResultsCount);
      for (int i = 0; i < transformResultsCount; i++) {
-        transformResults.add(decodeValue(proto.getTransformResults(i)));
+        transformResults.add(FieldValue.valueOf(proto.getTransformResults(i)));


Similarly: can we make transform results a model.ArrayValue so we don't have to convert?

schmidt-sebastian

Feedback addressed. I will work on a couple of follow-up PRs to address the newly added TODOs.

schmidt-sebastian · 2020-02-05T18:12:29Z

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java

@@ -533,7 +533,7 @@ private Object getInternal(
      if (val != null) {
        UserDataWriter userDataWriter =
            new UserDataWriter(firestore, timestampsInSnapshots, serverTimestampBehavior);
-        return userDataWriter.convertValue(val);
+        return userDataWriter.convertValue(val.getProto());


This pattern is used in two places in the code and in a bunch of test cases in UserWriterTest. I added a helper to UserDataWriterTest, but kept the usages in the code the same.

schmidt-sebastian · 2020-02-05T18:15:33Z

firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataReader.java

@@ -398,35 +389,37 @@ private void parseSentinelFieldValue(
   * @return The parsed value, or {@code null} if the value was a FieldValue sentinel that should
   *     not be included in the resulting parsed data.
   */
-  private FieldValue parseScalarValue(Object input, ParseContext context) {
+  private Value parseScalarValue(Object input, ParseContext context) {


I added a TODO for now. I would like to remove the individual field value wrappers first.

schmidt-sebastian · 2020-02-05T18:18:22Z

firebase-firestore/src/main/java/com/google/firebase/firestore/core/ArrayContainsAnyFilter.java

@@ -32,8 +32,8 @@ public boolean matches(Document doc) {
    if (!(other instanceof ArrayValue)) {
      return false;
    }
-    for (FieldValue val : ((ArrayValue) other).getInternalValue()) {
-      if (arrayValue.getInternalValue().contains(val)) {
+    for (FieldValue val : ((ArrayValue) other).getValues()) {


Done. More cleanup and the removal of ArrayValue will come in a follow-up.

schmidt-sebastian · 2020-02-05T18:24:07Z

firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java

@@ -165,7 +166,8 @@ public void setLocalDocumentsView(LocalDocumentsView localDocuments) {
  private static double estimateFilterSelectivity(Filter filter) {
    hardAssert(filter instanceof FieldFilter, "Filter type expected to be FieldFilter");
    FieldFilter fieldFilter = (FieldFilter) filter;
-    if (fieldFilter.getValue().equals(null) || fieldFilter.getValue().equals(DoubleValue.NaN)) {
+    FieldValue filterValue = fieldFilter.getValue();
+    if (NullValue.nullValue().equals(filterValue) || DoubleValue.NaN.equals(filterValue)) {


schmidt-sebastian · 2020-02-05T18:25:14Z

...-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java

        }
+        baseObject = baseObject.set(transform.getFieldPath(), coercedValue.getProto());


Yes, this is not needed anymore. We also don't use chaining anywhere - I wonder if we should remove it from the Builder API.

schmidt-sebastian · 2020-02-05T18:31:59Z

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

-    for (int i = 0; i < count; i++) {
-      result.add(decodeValue(elementsProto.getValues(i)));
+  private List<FieldValue> decodeArrayTransformElements(List<Value> elementsProto) {
+    List<FieldValue> result = new ArrayList<>(elementsProto.size());


schmidt-sebastian · 2020-02-05T18:32:55Z

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -648,7 +482,7 @@ public MutationResult decodeMutationResult(
    if (transformResultsCount > 0) {
      transformResults = new ArrayList<>(transformResultsCount);
      for (int i = 0; i < transformResultsCount; i++) {
-        transformResults.add(decodeValue(proto.getTransformResults(i)));
+        transformResults.add(FieldValue.valueOf(proto.getTransformResults(i)));


schmidt-sebastian · 2020-02-05T18:45:45Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -118,8 +127,13 @@ public void testAddsNewFields() {
  @Test
  public void testAddsMultipleNewFields() {
    ObjectValue object = ObjectValue.emptyObject();
-    object = object.toBuilder().set(field("a"), valueOf("a")).build();
-    object = object.toBuilder().set(field("b"), valueOf("b")).set(field("c"), valueOf("c")).build();
+    object = object.toBuilder().set(field("a"), wrap("a").getProto()).build();


Replaced this single occurrence with a call to the helper setField.

schmidt-sebastian · 2020-02-05T18:47:37Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -137,7 +151,7 @@ public void testImplicitlyCreatesObjects() {
  @Test
  public void testCanOverwritePrimitivesWithObjects() {
    ObjectValue old = wrapObject("a", map("b", "old"));
-    ObjectValue mod = setField(old, "a", map("b", "mod"));
+    ObjectValue mod = setField(old, "a", wrapObject("b", "mod"));


schmidt-sebastian · 2020-02-05T18:48:35Z

firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

-                field("foo.bar"),
-                new ServerTimestampValue(timestamp, StringValue.valueOf("bar-value")))
-            .build();
+    com.google.firebase.firestore.model.value.FieldValue fieldValue =


This test already imports com.google.firebase.firestore.FieldValue.

wilhuff

LGTM

googlebot added the cla: yes Override cla label Feb 1, 2020

google-oss-bot added the size/XXL label Feb 1, 2020

schmidt-sebastian force-pushed the mrschmidt/final branch 2 times, most recently from b85fba2 to 433da87 Compare February 3, 2020 23:56

schmidt-sebastian changed the title ~~Mrschmidt/final~~ Drop Protobuf-backed FieldValues Feb 3, 2020

schmidt-sebastian changed the base branch from mrschmidt/rewritefieldvalue to master February 3, 2020 23:57

schmidt-sebastian changed the base branch from master to mrschmidt/rewritefieldvalue February 3, 2020 23:57

schmidt-sebastian force-pushed the mrschmidt/final branch 2 times, most recently from cf6369a to 282380f Compare February 4, 2020 01:15

schmidt-sebastian requested a review from wilhuff February 4, 2020 01:15

schmidt-sebastian assigned wilhuff Feb 4, 2020

schmidt-sebastian added the api: firestore label Feb 4, 2020

schmidt-sebastian changed the title ~~Drop Protobuf-backed FieldValues~~ Protobuf-backed FieldValues Feb 4, 2020

Protobuf-backed FieldValues

1602062

schmidt-sebastian force-pushed the mrschmidt/final branch from 282380f to 1602062 Compare February 4, 2020 04:04

firebase deleted a comment from codecov bot Feb 4, 2020

schmidt-sebastian commented Feb 4, 2020

View reviewed changes

schmidt-sebastian added 2 commits February 4, 2020 13:39

Merge branch 'mrschmidt/rewritefieldvalue' into mrschmidt/final

804a1b7

Merge branch 'mrschmidt/rewritefieldvalue' into mrschmidt/final

3d5d7c2

wilhuff reviewed Feb 5, 2020

View reviewed changes

wilhuff assigned schmidt-sebastian and unassigned wilhuff Feb 5, 2020

Feedback

120ca33

schmidt-sebastian commented Feb 5, 2020

View reviewed changes

schmidt-sebastian added 2 commits February 5, 2020 11:03

Update test-only variable

82c0095

Add ProtoValues.contains()

98953a3

schmidt-sebastian assigned wilhuff Feb 6, 2020

wilhuff approved these changes Feb 6, 2020

View reviewed changes

wilhuff removed their assignment Feb 6, 2020

schmidt-sebastian merged commit f988c70 into mrschmidt/rewritefieldvalue Feb 6, 2020

schmidt-sebastian deleted the mrschmidt/final branch February 7, 2020 00:22

firebase locked and limited conversation to collaborators Mar 7, 2020

		}
		baseObject = baseObject.set(transform.getFieldPath(), coercedValue.getProto());

		@@ -167,8 +179,8 @@ public void testConvertsTimestampValue() {
		for (Timestamp d : testCases) {

Protobuf-backed FieldValues #1184

Protobuf-backed FieldValues #1184

Uh oh!

Conversation

schmidt-sebastian commented Feb 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

schmidt-sebastian Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilhuff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilhuff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

schmidt-sebastian commented Feb 1, 2020 •

edited

Loading

codecov bot commented Feb 4, 2020 •

edited

Loading

schmidt-sebastian Feb 4, 2020 •

edited

Loading