feat: Implement BundleBuilder #1041

wu-hui · 2020-04-23T18:22:08Z

Implements BundleBuilder and added system tests around bundles.

* Update from googleapis.git * Add bundle.proto. * Move bundle.proto to a different folder. * Move the proto file again * Update package name.

* Update from googleapis.git * Add bundle.proto. * Move bundle.proto to a different folder. * Add support to convert a Query to a BundledQuery. * Move the proto file again * Move the proto file again * Update package name. * Update to use new package name.

codecov · 2020-04-23T18:24:23Z

Codecov Report

Merging #1041 into wuandy/Bundles will increase coverage by 1.13%.
The diff coverage is 98.22%.

@@                Coverage Diff                 @@
##           wuandy/Bundles    #1041      +/-   ##
==================================================
+ Coverage           97.45%   98.59%   +1.13%     
==================================================
  Files                  25       26       +1     
  Lines               16044    16883     +839     
  Branches             1217     1342     +125     
==================================================
+ Hits                15636    16645    +1009     
+ Misses                405      235     -170     
  Partials                3        3

Impacted Files	Coverage Δ
dev/src/field-value.ts	`98.13% <85.71%> (+<0.01%)`	⬆️
dev/src/bundle.ts	`93.75% <93.75%> (ø)`
dev/src/types.ts	`99.69% <94.11%> (-0.31%)`	⬇️
dev/src/index.ts	`98.30% <94.44%> (-0.33%)`	⬇️
dev/src/document.ts	`98.97% <97.67%> (+0.31%)`	⬆️
dev/src/v1/firestore_client.ts	`97.64% <98.96%> (+2.84%)`	⬆️
dev/src/v1beta1/firestore_client.ts	`97.38% <99.02%> (+2.89%)`	⬆️
dev/src/v1/firestore_admin_client.ts	`97.71% <99.07%> (+6.58%)`	⬆️
dev/src/reference.ts	`99.88% <99.25%> (+0.15%)`	⬆️
dev/src/convert.ts	`98.27% <100.00%> (-0.03%)`	⬇️
... and 19 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 c119484...1d378fd. Read the comment docs.

schmidt-sebastian

I provided some initial feedback (have yet to look at the tests). Please note that system tests should be added on top of existing unit tests and not act as a replacement. It is much easier to detect failures and prescribe expected behavior with unit tests.

schmidt-sebastian · 2020-04-23T23:46:55Z

dev/src/bundle.ts

+import api = google.firestore.v1;
+
+/**
+ * Builds a Firestore data bundle with results from given queries and specified


Suggested change

* Builds a Firestore data bundle with results from given queries and specified

* Builds a Firestore data bundle with results from the given queries and specified

schmidt-sebastian · 2020-04-23T23:47:30Z

dev/src/bundle.ts

+ * Builds a Firestore data bundle with results from given queries and specified
+ * documents.
+ *
+ * For documents in scope for multiple queries, the latest read version will


Suggested change

* For documents in scope for multiple queries, the latest read version will

* For documents in scope for multiple queries, only the latest version will

schmidt-sebastian · 2020-04-23T23:47:51Z

dev/src/bundle.ts

+ * be included in the bundle.
+ */
+export class BundleBuilder {
+  // DocumentReference and Query added to the bundle.


Suggested change

// DocumentReference and Query added to the bundle.

// DocumentReferences and Queries added to the bundle.

schmidt-sebastian · 2020-04-23T23:48:14Z

dev/src/bundle.ts

+export class BundleBuilder {
+  // DocumentReference and Query added to the bundle.
+  private docRefs: DocumentReference[] = [];
+  private queries: Map<string, Query> = new Map<string, Query>();


Can you come up with a small inline doc string to describe the key of the Map?

schmidt-sebastian · 2020-04-23T23:48:28Z

dev/src/bundle.ts

+  private docRefs: DocumentReference[] = [];
+  private queries: Map<string, Query> = new Map<string, Query>();
+
+  // DocumentSnapshot and QuerySnapshot added to the bundle.


Suggested change

// DocumentSnapshot and QuerySnapshot added to the bundle.

// DocumentSnapshots and QuerySnapshots added to the bundle.

schmidt-sebastian · 2020-04-24T00:10:00Z

dev/src/bundle.ts

+    }
+
+    for (const [name, query] of Array.from(this.queries)) {
+      if (this.namedQueries.has(name)) {


This should be part of the add call.

schmidt-sebastian · 2020-04-24T00:11:30Z

dev/src/bundle.ts

+
+    const readable = new Readable({
+      objectMode: false,
+      read: async size => {


Read cannot be async as far as I can tell. It needs to return the data that is being read.

Note this is the read from ReadableOptions, and its return type is a void.

However I will switch to asyncIterator anyways, so it does not matter any more.

schmidt-sebastian · 2020-04-24T00:15:36Z

dev/src/bundle.ts

+        };
+        this.pushLengthPrefixedString(
+          readable,
+          JSON.stringify({metadata} as firestore.IBundleElement)


The cast is not valid (since metadata is IBundleMetadata, but not {metadata}). You can just drop the cast.

I think it is valid but unnecessary, because it is casting to IBundleElement.

I slightly prefer to keep these though, because it can serve as a reminder that it is a IBundleElement that gets saved in the bundle.

dev/src/bundle.ts

dev/src/index.ts

schmidt-sebastian · 2020-04-28T20:51:09Z

dev/src/bundle.ts

+
+/**
+ * Builds a Firestore data bundle with results from the given queries and specified
+ * documents.


We should eventually expand on the docs to say how bundles can be used. This can be deferred until we have a client SDK implemented.

dev/src/bundle.ts

schmidt-sebastian · 2020-04-28T20:52:38Z

dev/src/bundle.ts

+  constructor(private bundleId: string) {}
+
+  add(documentSnapshot: DocumentSnapshot): BundleBuilder;
+  add(queryName: string, querySnap: QuerySnapshot): BundleBuilder;


s/querySnap/querySnapshot/ for consistency.

schmidt-sebastian · 2020-04-28T20:53:04Z

dev/src/bundle.ts

+   * Both the documents data and the query read time will be included in the bundle.
+   *
+   * @param {DocumentSnapshot | string=} documentOrName A document snapshot to add or a name of a query.
+   * @param {Query?=} querySnapshot A query snapshot to add to the bundle, if provided.


s/Query?=/Query=

JSDoc for undefined is just =

schmidt-sebastian · 2020-04-28T20:54:28Z

dev/src/bundle.ts

+   * @example
+   * let bundle = firestore.bundle('data-bundle');
+   * const bundleStream =
+   *    await bundle.add(await firestore.doc('abc/123').get()) // Add a document


Can you change this to move the calls to bundle.add() to their own lines?
Also, you should not need to use await here.

schmidt-sebastian · 2020-04-28T21:27:39Z

dev/system-test/firestore.ts

+
+describe('Bundle building', () => {
+  // Helper function for testing: converts "13{..}43{..}51{..}" to [[13,43,51],[obj,obj,obj]]
+  function bundleToLengthsAndObject(bundle: string): [number[], object[]] {


To make this easier to digest, consider returning an array with objects: Array<{length: number, content: IBundleElement}>

schmidt-sebastian · 2020-04-28T21:35:05Z

dev/system-test/firestore.ts

+    let insertComma = false;
+
+    // Turn "13{..}43{..}51{..}" into "{..},{..},{..}"
+    for (const c of bundle) {


This is a surprising way to read this data. You should take advantage of the data format you build and read this in two phases:

Extract the length by reading until you hit "{".

Reading the next "length" bytes

I want to avoid that actually. What you described is the way client SDKs would be using. I want the tests to be using different (inefficient) ways such that two implementations can cross check (not obvious here, but I imagine this function will be copied over to web SDK).

I haven't yet convinced myself that the logic here adds more test coverage, but I am convinced that it is much harder to parse than the code that we expect users to write would be. In fact, if we don't think that our users will parse bundles like this, then we shouldn't either. We need to validate that the format we are using doesn't just match our specification, but can actually be consumed as intended by our users.

You could use this function in one (or if you want to maybe even most) unit tests, but you should at least have some coverage that uses the "straightforward" way to parse this data. FWIW, there are even packages that you can use: https://www.npmjs.com/package/length-prefixed-json-stream

Done, thanks!

schmidt-sebastian · 2020-04-28T21:39:07Z

dev/system-test/firestore.ts

+
+  afterEach(() => verifyInstance(firestore));
+
+  it('succeeds to read length prefixed json with testing function', () => {


Should we make this a unit test and move bundleToLengthsAndObject to helpers.ts?

The reason I keep it here is because this is only used in tests, not prod code.

This particular one is definitely a unit test, but because it is only used here in the system tests, we would like to keep them together. It's easier to people to read IMO.

As I suggested below, maybe grouping all bundle system tests (and this one) in a separate file will help?

The helpers.ts is actually a unit test-only file: https://github.com/googleapis/nodejs-firestore/blob/master/dev/test/util/helpers.ts

My apologies..done.

dev/system-test/firestore.ts

schmidt-sebastian · 2020-04-28T21:48:19Z

dev/system-test/firestore.ts

+    ]);
+  });
+
+  it('succeeds with document snapshots', async () => {


All of these tests should be unit tests in their own file (bundle.ts). We can cherry-pick some tests and duplicate them as system tests.

We also need at least one test that consumes stream() directly.

I actually think in this case, integration tests are easier to write than unit tests. All snapshots are constructed nicely, with read time populated correctly. Unit tests are be harder to read IMHO.

Do you keep them as system tests but move them to a separate file will make it better?

System tests are usually easier to write, but they:

are harder to debug

are not deterministic

make it impossible to test some edge cases

cannot be run without Production credentials

don't count against code coverage

Please add unit tests :)

Moved some to unit tests, thanks.

schmidt-sebastian · 2020-04-30T16:34:11Z

dev/src/bundle.ts


 import api = google.firestore.v1;

 /**
 * Builds a Firestore data bundle with results from the given queries and specified
 * documents.
 *
- * For documents in scope for multiple queries, only the latest version will
+ * For documents that are included multiple times, only the latest version will


Independent of how we decide to implement this, should we relax this documentation to allow us to send multiple document version if the are processed "oldest to newest" first?

Removed this section.

schmidt-sebastian · 2020-04-30T16:35:30Z

dev/src/bundle.ts

+   * const querySnapshot = await firestore.collection('coll').get();
+   * const bundleStream = bundle.add(docSnapshot) // Add a document
+   *                            .add('coll-query', querySnapshot) // Add a named query.
+   *                            .stream();


Suggested change

* .stream();

* bundle.add(docSnapshot) // Add a document

* bundle.add('coll-query', querySnapshot) // Add a named query.

* const bundleStream.stream();

schmidt-sebastian · 2020-04-30T16:37:48Z

dev/src/bundle.ts

-    if (documentOrName instanceof DocumentSnapshot) {
+    if (
+      documentOrName instanceof DocumentSnapshot &&
+      validateOptional(querySnapshot, {optional: true})


validateOptional only validates that querySnapshot can be undefined not that it needs to be undefined when documentOrName is a DocumentSnapshot.

This validation should also be moved into the if branch.

Rewrote to check arguments.length instead..I found it to be more readable.

dev/src/bundle.ts

schmidt-sebastian · 2020-04-30T16:41:08Z

dev/src/bundle.ts

@@ -113,8 +118,12 @@ export class BundleBuilder {
    }
  }

-  private lengthPrefixedBuffer(payload: string): Buffer {
-    const buffer = Buffer.from(payload, 'utf-8');
+  // Converts a IBundleElement to a Buffer whose content is the length prefixed JSON representation


For method comments, prefer JSDoc:

/** * Converts a IBundleElement... * @private */

Note that you have to add @private to make sure this doesn't show up in our public docs. JSDoc doesn't know that this method is TypeScript private.

dev/src/bundle.ts

schmidt-sebastian · 2020-04-30T16:43:01Z

dev/system-test/firestore.ts

+
+  afterEach(() => verifyInstance(firestore));
+
+  it('succeeds to read length prefixed json with testing function', () => {


The helpers.ts is actually a unit test-only file: https://github.com/googleapis/nodejs-firestore/blob/master/dev/test/util/helpers.ts

schmidt-sebastian · 2020-04-30T16:49:54Z

dev/system-test/firestore.ts

+    let insertComma = false;
+
+    // Turn "13{..}43{..}51{..}" into "{..},{..},{..}"
+    for (const c of bundle) {


I haven't yet convinced myself that the logic here adds more test coverage, but I am convinced that it is much harder to parse than the code that we expect users to write would be. In fact, if we don't think that our users will parse bundles like this, then we shouldn't either. We need to validate that the format we are using doesn't just match our specification, but can actually be consumed as intended by our users.

You could use this function in one (or if you want to maybe even most) unit tests, but you should at least have some coverage that uses the "straightforward" way to parse this data. FWIW, there are even packages that you can use: https://www.npmjs.com/package/length-prefixed-json-stream

schmidt-sebastian

I have some belated API suggestions.... but otherwise this looks good.

schmidt-sebastian · 2020-05-06T04:39:06Z

dev/src/bundle.ts

+ * @param arg The argument name or argument index (for varargs methods).
+ * @param value The input to validate.
+ */
+export function validateDocumentSnapshot(


Nit: No need to export. We generally also have helpers at the bottom of the model.

schmidt-sebastian · 2020-05-06T04:39:28Z

dev/src/bundle.ts

+ * @param arg The argument name or argument index (for varargs methods).
+ * @param value The input to validate.
+ */
+export function validateQuerySnapshot(


Same comment as above.

schmidt-sebastian · 2020-05-06T04:40:00Z

dev/src/bundle.ts

+   * Adds a Firestore document snapshot or query snapshot to the bundle.
+   * Both the documents data and the query read time will be included in the bundle.
+   *
+   * @param {DocumentSnapshot | string=} documentOrName A document snapshot to add or a name of a query.


s/string=/string/

schmidt-sebastian · 2020-05-06T04:40:21Z

dev/src/bundle.ts

+   * @returns {BundleBuilder} This instance.
+   *
+   * @example
+   * let bundle = firestore.bundle('data-bundle');


s/let/const/

schmidt-sebastian · 2020-05-06T04:40:57Z

dev/src/bundle.ts

+   * const querySnapshot = await firestore.collection('coll').get();
+   * bundle.add(docSnapshot); // Add a document.
+   * bundle.add('coll-query', querySnapshot) // Add a named query.
+   * const bundleBuffer = await bundle.build();


This doesn't need to be "awaited" anymore.

dev/system-test/firestore.ts

schmidt-sebastian · 2020-05-06T05:01:01Z

dev/test/bundle.ts

+        readTime: snap1.readTime.toProto().timestampValue,
+      },
+      snap1.toDocumentProto(),
+    ]);


This seems like it should be two different expect calls.

schmidt-sebastian · 2020-05-06T05:03:25Z

dev/test/bundle.ts

+      snap1.toDocumentProto(),
+    ]);
+  });
+


Should we add a test that a second call to build() contains the first round of documents as well as the ones added after the first call?

schmidt-sebastian · 2020-05-06T05:04:53Z

dev/test/util/helpers.ts

+  bundle: Buffer
+): Promise<Array<firestore.IBundleElement>> {
+  const result: Array<firestore.IBundleElement> = [];
+  const readable = new PassThrough();


Suggested change

const readable = new PassThrough();

const readable = Readable.from(bundle);

When i try to use from i get "TypeError: stream_1.Readable.from is not a function". Not sure what is wrong.

Also, from the node documentation: "Calling Readable.from(string) or Readable.from(buffer) will not have the strings or buffers be iterated to match the other streams semantics for performance reasons.".

schmidt-sebastian · 2020-05-06T05:05:30Z

dev/src/external-modules.d.ts

@@ -16,3 +16,4 @@

 // TODO(mrschmidt): Come up with actual definitions for these modules.
 declare module 'functional-red-black-tree';
+declare module 'length-prefixed-json-stream';


I see what you did there, adding work to my TODO :)

Damn, I should have picked someone else.

schmidt-sebastian · 2020-05-06T22:20:32Z

dev/src/document.ts

-      name: this._ref.formattedName,
-      fields: this._fieldsProto!,
-    };
+    const result: api.IDocument = {name: this._ref.formattedName};


I think you should do this conversion in the caller. The backend has a specified way to return "NoDocuments": https://source.corp.google.com/piper///depot/google3/google/firestore/v1beta1/firestore.proto;l=496

In the calling code, you could do something like

if (doc.exists) ? { document: doc.toProto()} : {name: this._ref.formattedName}

In the Mobile SDKs, you want to use as much of the existing deserialization logic as possible. For that, you want to match your Proto format. From looking at your Proto, it seems that you could also add an exists to IBundledDocumentMetadata and omit the document?

Drive by comment on Proto: We may also want to use "name" for document keys since this is what the backend uses.

Added exists and changed to name.

dev/system-test/firestore.ts

schmidt-sebastian · 2020-05-06T22:23:20Z

dev/test/bundle.ts

@@ -24,7 +24,7 @@ import {
 } from './util/helpers';
 import IBundleElement = firestore.IBundleElement;

-describe('Bundle Buidler', () => {
+describe.only('Bundle Buidler', () => {


If you merge @thebrianchen's changes form the Node10 branch into your PR you will get a lint check that will catch this :)

There is a lot changes from bulk writer too. I will do the merge later..

schmidt-sebastian · 2020-05-06T22:24:07Z

dev/src/bundle.ts

-  arg: string | number,
-  value: unknown
-): void {
+function validateQuerySnapshot(arg: string | number, value: unknown): void {


Please do move to bottom of the module to match the code layout in the rest of the repo.

wu-hui

Also added a version to metadata.

wu-hui · 2020-05-07T14:23:50Z

dev/src/bundle.ts

-  arg: string | number,
-  value: unknown
-): void {
+function validateQuerySnapshot(arg: string | number, value: unknown): void {


wu-hui · 2020-05-07T14:45:42Z

dev/src/document.ts

-      name: this._ref.formattedName,
-      fields: this._fieldsProto!,
-    };
+    const result: api.IDocument = {name: this._ref.formattedName};


Added exists and changed to name.

wu-hui · 2020-05-07T15:20:08Z

dev/test/bundle.ts

@@ -24,7 +24,7 @@ import {
 } from './util/helpers';
 import IBundleElement = firestore.IBundleElement;

-describe('Bundle Buidler', () => {
+describe.only('Bundle Buidler', () => {


There is a lot changes from bulk writer too. I will do the merge later..

wu-hui · 2020-05-07T16:45:07Z

Also added a version to metadata.

One more thing: the change to make it non-public will be separate, such that it can be easily reverted.

schmidt-sebastian

LGTM minus missing unit test for "addNamedQuery".

schmidt-sebastian · 2020-05-07T17:47:14Z

dev/protos/firestore/bundle.proto

@@ -85,6 +88,9 @@ message BundleMetadata {

  // Time at which the documents snapshot is taken for this bundle.
  google.protobuf.Timestamp create_time = 2;
+
+  // The version of bundle.


Nit: "The schema version of the bundle."

schmidt-sebastian · 2020-05-07T17:48:24Z

dev/src/bundle.ts

@@ -109,10 +85,11 @@ export class BundleBuilder {
  private addBundledDocument(snap: DocumentSnapshot) {
    const docProto = snap.toDocumentProto();
    this.documents.set(snap.id, {
-      document: docProto,
+      document: snap.exists ? docProto : undefined,


Nit: I don't know what snap.toDocumentProto() does if the document doesn't exist. I would not need to check if you called it here inline. :)

Optional.

there is another docProto.name below, so I prefer to keeping it this way.

schmidt-sebastian · 2020-05-07T17:49:32Z

dev/src/bundle.ts

@@ -174,16 +152,19 @@ export class BundleBuilder {
    for (const bundledDocument of this.documents.values()) {
      const documentMetadata: firestore.IBundledDocumentMetadata =
        bundledDocument.metadata;
-      const document: api.IDocument = bundledDocument.document;
+      const document = bundledDocument.document;


Nit: Move to line 162.

schmidt-sebastian

Thanks for adding the test. Future you (and me) will thank you again.

JustinBeckwith and others added 5 commits April 12, 2020 02:16

build: remove unused codecov config (#1011)

60856aa

chore: update lint ignore files (#1015)

ce89a30

feat: add bundle.proto and generated js code (#1032)

4dac6d1

* Update from googleapis.git * Add bundle.proto. * Move bundle.proto to a different folder. * Move the proto file again * Update package name.

Implementation and tests.

30671a6

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 23, 2020

wu-hui added 2 commits April 23, 2020 15:01

More readable.

d666d48

proper license header.

2617786

wu-hui requested a review from schmidt-sebastian April 23, 2020 19:08

wu-hui assigned schmidt-sebastian Apr 23, 2020

schmidt-sebastian reviewed Apr 24, 2020

View reviewed changes

schmidt-sebastian assigned wu-hui Apr 24, 2020

Address comments 1

2951e04

wu-hui force-pushed the wuandy/Bundles branch 2 times, most recently from 5023791 to c119484 Compare April 24, 2020 21:32

wu-hui added 2 commits April 24, 2020 17:51

Merge branch 'wuandy/Bundles' into wuandy/BuildBundles

9d7e9b1

Address comments 2

7b54ddd

wu-hui removed their assignment Apr 27, 2020

schmidt-sebastian suggested changes Apr 28, 2020

View reviewed changes

wu-hui assigned wu-hui and unassigned schmidt-sebastian Apr 30, 2020

Address comments 3

daa89f8

wu-hui assigned schmidt-sebastian and unassigned wu-hui Apr 30, 2020

schmidt-sebastian reviewed Apr 30, 2020

View reviewed changes

schmidt-sebastian assigned wu-hui and unassigned schmidt-sebastian Apr 30, 2020

Address comments 4 + Remove stream() from bundle builder

44750aa

wu-hui removed their assignment May 5, 2020

schmidt-sebastian reviewed May 6, 2020

View reviewed changes

schmidt-sebastian assigned thebrianchen and wu-hui and unassigned schmidt-sebastian and thebrianchen May 6, 2020

Address comments 5

5b6cea3

wu-hui assigned schmidt-sebastian and unassigned wu-hui May 6, 2020

schmidt-sebastian reviewed May 6, 2020

View reviewed changes

schmidt-sebastian assigned thebrianchen and wu-hui and unassigned thebrianchen and schmidt-sebastian May 6, 2020

More tweaks.

7c83476

wu-hui commented May 7, 2020

View reviewed changes

wu-hui assigned schmidt-sebastian and unassigned wu-hui May 7, 2020

schmidt-sebastian suggested changes May 7, 2020

View reviewed changes

schmidt-sebastian assigned wu-hui and unassigned schmidt-sebastian May 7, 2020

wu-hui added 2 commits May 7, 2020 14:27

More comments

66c3cb9

Add one more unit test

1d378fd

wu-hui assigned schmidt-sebastian and unassigned wu-hui May 7, 2020

schmidt-sebastian approved these changes May 7, 2020

View reviewed changes

schmidt-sebastian assigned wu-hui and unassigned schmidt-sebastian May 7, 2020

wu-hui merged commit 98f58eb into wuandy/Bundles May 7, 2020

wu-hui added a commit that referenced this pull request May 13, 2020

feat: Implement BundleBuilder (#1041)

b57b87a

	* Builds a Firestore data bundle with results from given queries and specified
	* Builds a Firestore data bundle with results from the given queries and specified

	* For documents in scope for multiple queries, the latest read version will
	* For documents in scope for multiple queries, only the latest version will

	// DocumentReference and Query added to the bundle.
	// DocumentReferences and Queries added to the bundle.

	// DocumentSnapshot and QuerySnapshot added to the bundle.
	// DocumentSnapshots and QuerySnapshots added to the bundle.


		afterEach(() => verifyInstance(firestore));

		it('succeeds to read length prefixed json with testing function', () => {

	const readable = new PassThrough();
	const readable = Readable.from(bundle);

feat: Implement BundleBuilder #1041

feat: Implement BundleBuilder #1041

Conversation

wu-hui commented Apr 23, 2020

codecov bot commented Apr 23, 2020 • edited Loading

Codecov Report

schmidt-sebastian left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 23, 2020 •

edited

Loading

schmidt-sebastian left a comment •

edited

Loading