refactor(typescript): upgrade to TS 4.1, simplify and generalize types #480

eXamadeus · 2021-01-30T10:08:34Z

EDIT:

See #480 (comment) for "status" tracker of the changes suggested by everyone

Also, when I updated my master branch it purged this PR of the code. The latest commit for reference was 66fcf90. You can see the original changes @ eXamadeus#3 or #487.

What

Upgrades to the latest version of TypeScript: 4.1. I think this will be breaking changes. I mean it isn't a change to the code, but the TS updates might require some slight modifications to user code if they were using the ParametricSelector or Selector types directly.

Changes

types no longer suffer "death from a thousand overloads"
createSelector can accept any number of selectors
- the limit is only in the depth of recursion TS allows, which is 50 levels as of now
- why this arbitrary number? I dunno ask the TS guys
removed ParametricSelector as the base Selector type is now smart enough to "encompass" this role

CI/Build Chores

updated Travis targets to still maintained versions of node
fixed broken build scripts
updated TypeScript testing strategy to use https://github.com/microsoft/dtslint
- created /types folder and moved tests there
- added symlink to test folder (easiest way to setup new test)

codecov · 2021-01-30T10:14:09Z

Codecov Report

Merging #480 (66fcf90) into master (0c25739) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #480   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           15        15           
=========================================
  Hits            15        15

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 e93fe66...66fcf90. Read the comment docs.

eXamadeus · 2021-01-30T12:00:14Z

.travis.yml

+      - "10"
+      - "12"
+      - "14"
+      - "15"


The node versions here were ancient. I updated them because the new test tooling just simply won't run in old unsupported versions.

eXamadeus · 2021-01-30T12:00:38Z

package.json

@@ -17,7 +17,7 @@
  },
  "scripts": {
    "compile:commonjs": "better-npm-run compile:commonjs",
-    "compile:umdmin": "uglifyjs dist/reselect.js -mt -o dist/reselect.min.js",
+    "compile:umdmin": "uglifyjs dist/reselect.js -m -o dist/reselect.min.js",


The -t flag is not valid, so I removed it.

eXamadeus · 2021-01-30T12:01:10Z

package.json

@@ -42,7 +42,7 @@
      }
    },
    "test:typescript": {
-      "command": "typings-tester --dir typescript_test"
+      "command": "dtslint --expectOnly types"


This is a superior testing suite to typings-tester. This is also the officially supported type testing module by the TypeScript team.

eXamadeus · 2021-01-30T12:04:23Z

types/index.d.ts

@@ -0,0 +1 @@
+../src/index.d.ts


This is a symlink...unfortunately dtslint is pretty annoying with it's lack of configurability.

In order to get the tests to run, you need the type definitions and the "tests" to be located in the same directory. Since I didn't want to touch the main /src directory, since it's part of the bundling path, I decided to just make a simple symlink here to let the tests run against the main type definitions.

It all works, but I would understand if there was pushback here. Although I do think that git-based symlinks work on every OS nowadays.

eXamadeus · 2021-01-30T12:04:56Z

types/test.ts

+declare function connect<S, R, P>(selector: Selector<S, R, P[]>):
+  (component: Component<R & S & ([P] extends [never] ? {} : P)>) => Component<P>;


The Selector types were modified, so we needed to update this helper

I noticed that R and P are swapped here. Would that impact back compatibility (positively or negatively)?

Not here in the test, but it might cause issues in client usage of Selector. This is something that I should revert though.

I changed the order without realizing because I rewrote the types from scratch. I'm definitely going to fix the order here, since it's a pointless change that would just inconvenience people.

eXamadeus · 2021-01-30T12:05:21Z

types/test.ts


 function testConnect() {
  connect(
    createSelector(
      (state: {foo: string}) => state.foo,
-      foo => ({foo}),
+      foo => ({changeUp: foo}),


This made the test more obvious that it was "changing" the return type values to something different.

eXamadeus · 2021-01-30T12:06:57Z

types/test.ts

  });

  const connected = connect(
    createSelector(
      (state: {foo: string}) => state.foo,
-      (state: never, props: {bar: number}) => props.bar,
-      (foo, bar) => ({foo, baz: bar}),
+      (state: {foo: string}, props: {bar: number}) => props.bar,


never is now a proper bottom type and will not assign across a heterogenous collection of selectors.

The state object should always be the same regardless of the selector, so TypeScript is doing us a favor here by forcing users to use a heterogenous State type or any.

eXamadeus · 2021-01-30T12:08:23Z

types/test.ts

+      (foo, bar) => ({foo, baz: !!bar}),
    )
  )(props => {
    const foo: string = props.foo;
    const bar: number = props.bar;
-    const baz: number = props.baz;
-    // typings:expect-error
+    const baz: boolean = props.baz;


Made the conversion of types clearer here for future developers.

eXamadeus · 2021-01-30T12:09:17Z

types/test.ts

+    (state: any) => state.foo,
+    (state: any) => state.foo,
+    (state: any) => state.foo,
+    (state: any) => state.foo,
+    (state: any) => state.foo,


Non-explicit any is generally disallowed, we will enforce this via our type definitions. This can be disabled with a tsconfig.json change on a user-by-user basis.

eXamadeus · 2021-01-30T12:10:08Z

types/test.ts

@@ -225,20 +228,20 @@ function testArrayArgument() {
  const selector = createSelector([
    (state: {foo: string}) => state.foo,
    (state: {foo: string}) => state.foo,
-    (state: never, props: {bar: number}) => props.bar,
+    (state, props: {bar: number}) => props.bar,


Implicit any values can be used in place of never for heterogenous State selectors.

eXamadeus · 2021-01-30T12:10:36Z

types/test.ts

@@ -387,7 +390,7 @@ function testDefaultMemoize() {
      if (index === 0)
        return a === b;

-      return a.toString() === b.toString();
+      return `${a}` === `${b}`;


TypeScript bug that was fixed...

markerikson · 2021-01-30T17:00:05Z

Wow, thanks for putting this together!

The biggest issue atm is that @ellbee , the primary maintainer for Reselect, has been mostly inactive for a while. Tim Dorr and I have commit rights to the repo as owners of the reduxjs GH org, but we don't have publish rights on NPM. So, even if we merge this, we don't currently have a way to publish a new build.

I'm going to try reaching out to @ellbee and seeing if we can get added as publishers.

phryneas

I added some comments. I don't think this needs a new type testing framework honestly, if anything the testing framework could be removed in the long run if we were to target only 3.9 and above.
Right now, redux toolkit, which depends on reselect goes down to 3.5, so that should probably stay supported for now though.

As for backwards compatibility: I'd suggest keeping existing types with their names as they are and deprecating them.
In addition to that, I'd keep the old type definition in a second file and use typesVersions to have TS choose the new one only for TS >=4.1

phryneas · 2021-01-30T17:54:04Z

types/test.ts

@@ -20,10 +20,10 @@ function testSelector() {

  const foo: string = selector({foo: 'bar'});

-  // typings:expect-error
+  // $ExpectError


If this only needs to be tested on more modern versions of TS, this could also just be // @ts-expect-error - that way no test framework would be required at all.

Yeah that makes sense. I would prefer to use the // @ts-expect-error directive. If we need to support 3.5 for redux-toolkit, we probably need to keep the // $ExpectError directive though.

The main reason that I upgraded from typings-tester was for compatibility reasons with later versions of TypeScript and for somewhat selfish development reasons. The error messages from dtslint were very helpful in making sure I covered all the edge cases correctly in the new typings. Seeing a descriptive error in the output was a lot easier than having to just see "error @ line ###".

I don't mind downgrading now, if you think it's important. But if we do want to support things < 3.9 we'll need some sort of tooling to help with that.

One other note, I think the main reason that something like dtslint is useful is that it tests for type compatibility across a range of TS versions where the // @ts-expect-error directive only works for the installed version, right?

I think that would be a compelling reason to actually keep something like dtslint around, right?

Over on redux toolkit, we use a test matrix that just runs tsc on the files with each tested TypeScript version.
https://github.com/reduxjs/redux-toolkit/blob/next/.github/workflows/tests.yml

The nice thing about // @ts-expect-error is the usage in the editor. If there is an error where one is expected, everything is fine. If the error is absent, you get squiggly lines. So you can just look into the editor to spot where your test breaks, you don't even need to run the tests.

Especially for writing the tests, that's a big thing.

Oh, that's a good idea! I think we can do something similar with TravisCI here right?

If I have the go-ahead to do that, I don't think there are any differences between 3.5 and 3.9 (in regards to this library) so we could just say that this only supports 3.5 and above while only testing 3.9 + using the // @ts-expect-error comments.

Basically, we can validate that 3.5, 3.6, 3.7, and 3.8 work with these typings (we know they do because of all the tests we've been running in this PR) but only setup automation to check 3.9 and above. I guess we could say this is "verified again 3.5-3.8, and actively tested against 3.9+".

I'm super new to OSS library maintenance, so I'm down for whatever you guys suggest. You're the experts haha.

Or I can still do the matrix and use the // $ExpectError directive for 3.5+ and // @ts-expect-error for the new typings. Either way seems good to me. But the removal of the testing library seems like a more "future-proof" solution.

A build matrix would be possible with Travis, but since Travis CI is pretty much kicking out open source projects by restricting build times & becoming more & more unreliable, it would probably be a good idea to switch over to GH actions anyways.

As for testing older TS versions than 3.9... I'd be fine not to, since I don't expect any bigger changes coming up in the near future and the current state has already been tested for a long time. @markerikson what do you think about this?

If @markerikson is good with it, I'll do the switch over to GH actions here too. It's the way of the future anyway, right? 😆

I could also totally break that up into another PR. Just let me know what you would prefer.

As for testing older TS versions than 3.9... I'd be fine not to, since I don't expect any bigger changes coming up in the near future and the current state has already been tested for a long time.

That would also make the testing a lot easier because you wouldn't need to do the swapping of the test comments or maintain separate type files. I was looking at the way you guys did it in the toolkit and that last bit of the job looked a little gnarly with all the sed stuff.

phryneas · 2021-01-30T18:10:17Z

src/index.d.ts


-export type OutputParametricSelector<S, P, R, C> = ParametricSelector<S, P, R> & {
-  resultFunc: C;
+export type Selector<S = any, R = unknown, P extends never | (readonly any[]) = any[]> = [P] extends [never] ? (state: S) => R : (state: S, ...params: P) => R;


My suggestion would be to keep Selector, ParametricSelector and OutputParametricSelector in as it they are, mark them as @deprecated and use a new name for this type to avoid a breaking change.

Hmm, so I think if I address the suggestion here these will be backward compatible right?

This is because the new Selector is just a combined version of the old Selector and Parametric Selector. The only thing that could be considered breaking, in my opinion, is the "extra args" bit of the ParametricSelectors. Those were always just passed through as ...any[] where they are not fully typed. In every other instance, I think these types will behave exactly the same, no?

See this playground link for an example of what I'm talking about. I'm on the fence if this would be considered a bug fix or a breaking change. In reality, it's kind of a bug fix right? If they do provide extra args in any of the selectors, they will behave the same and be typed more correctly than before.

src/index.d.ts

phryneas · 2021-01-30T18:13:12Z

src/index.d.ts

+    : []
+  : Rest extends ((...args: any) => infer S)[] ? S[] : [];
+
+export function createSelector<Selectors extends SelectorArray, Result>(


The breaking change in the arguments to createSelector would be kinda expected for me - I'd be okay with that.

src/index.d.ts

eXamadeus · 2021-01-30T19:18:25Z

As for backwards compatibility: I'd suggest keeping existing types with their names as they are and deprecating them.
In addition to that, I'd keep the old type definition in a second file and use typesVersions to have TS choose the new one only for TS >=4.1

I'm embarrassed to admit that I had no idea that could be done. I've never written package typings before so that thought never crossed my mind. I will absolutely do this though!

eXamadeus · 2021-02-06T01:59:11Z

Just as an update, I plan to get around to updating this with the suggestions this weekend. I'm sorry I didn't reply/update this sooner, it's been a busy week at work.

I am very thankful for the reviews though. I also plan to resolve those and make corrections in my code as soon as I am able.

markerikson · 2021-02-07T19:50:57Z

Hey, wanted to follow up on this.

I'd like to split the work in this PR into a couple different PRs if at all possible.

First, per #482 , I definitely want to modernize the CI setup, including switching from TravisCI to Github Actions and dropping old versions of Node. However, I think that should be its own PR.

From there, yes, I do want to update any dependencies and actual TS typings.

Not sure if it'd be easier to close this PR and then create two new ones?

markerikson · 2021-02-07T19:56:33Z

Also: my other concern atm is that it looks like some of the changes in here are probably enough to justify a Reselect 5.0 release. However, if we do a 5.0, it seems like there are also a number of other functionality changes that should be considered as well, such as #401 , and even rewriting the code itself in TS.

Meanwhile, there's open PRs like #465 , which seems small but useful, and could be put out in a new 4.x release.

Given that, can we figure out a way to scope the changes so that we can have a 4.x release soon with some additional bits of cleanup, and then do a larger effort to work towards a 5.x?

eXamadeus · 2021-02-07T22:12:55Z

Given that, can we figure out a way to scope the changes so that we can have a 4.x release soon with some additional bits of cleanup, and then do a larger effort to work towards a 5.x?

Sure. I really like that idea. We can do a 4.x PR that is only non-breaking upgrades. A non-exhaustive list:

CI upgrade from Travis -> GHA
Use Callable Type Signature for Memoize in createSelectorCreator #465
...whatever else looks good to pull into a 4.x release.

Then I can start a 5.0 release w/:

the 4.1 type updates I did here
some incarnation of Version 5.0.0 proposal #401?

I would love to include #401 (well some variant of it) in the 5.0 release PR, but I'm not sure there is a consensus on the names for the new API. I have read through all the posts and related issues (that took a while BTW 😆) and think it's a great idea. I'm of the opinion it shouldn't be called createKeySelector (which I think @josepot agrees with).

My personal opinion is that it should be called createDynamicSelector AND that it should still be able to accept the state. The "dynamic selector" moniker would just let people know that it won't be considered for caching purposes. This is a slightly less "leaky" way to name it. What do you think @josepot? createDynamicSelector should still behave the same as createKeySelector and will still allow for things like shared selectors. Heck, it might even enable things with can't think of right now. I am not sure of any downsides to still allowing the state. I think most people wouldn't use it, but I don't see why it would be a problem either.

I have no problem making both of these PRs. Or I can contribute to them if you'd rather control that process. Just let me know.

markerikson · 2021-02-07T22:32:56Z

@eXamadeus yep, sounds good. Let's do the build updates first. In fact, let's split this up into separate PRs for :

CI changes
Dependency updates
Any minor fixes we feel are appropriate for a 4.x release.

I think it's worth having a larger planning discussion around what 5.x should include. There's a bunch of open PRs that I could see conflicting. I think it's also important to analyze what weaknesses Reselect currently has and what use cases it doesn't convert well atm, and look at other alternatives in the ecosystem for comparison.

Related to all this, I've been looking at https://github.com/dai-shi/proxy-memoize as a possible better alternative to Reselect that we could start recommending. Discussion thread for that topic is at reduxjs/react-redux#1653.

However, I'm also interested in improving Reselect itself too. If we're going to do so, I want to have a plan laid out.

I'll try to create a discussion thread tonight or tomorrow.

eXamadeus · 2021-02-07T23:39:10Z

@markerikson Sounds good, I'll have them up tonight or tomorrow (at least the CI one for sure).

Question about the dependency updates: which updates does reselect want to entertain? If we decided to just assume TS 3.9+ support we can drop the testing libraries completely. If that's the route we want to go, that's probably the easiest one. Also, do you want to do a generic upgrade of other dependencies while were at it? Or just remove the testing libraries and update to say 3.9+ is supported?

Either way, I can create the CI PR and a "minor fixes" PR that follows that w/ #465 in it.

markerikson · 2021-02-08T01:58:09Z

Yeah, I still want to do this in phases. There were several open "update deps" PRs I closed today. Like, this is apparently still on Babel 6 (!).

I think we can certainly ignore TS < 3.5, and it's probably not unreasonable to go 3.9+. Of course, as we just saw, 3.1+ appears to be an issue :)

eXamadeus · 2021-02-08T02:56:31Z

I think we can certainly ignore TS < 3.5, and it's probably not unreasonable to go 3.9+. Of course, as we just saw, 3.1+ appears to be an issue :)

Yeah, it's only two of the tests that are broken (sort of). The actual library in production is already affected by this, so it's not like removing the tests would cause a breaking change. It's basically just acknowledging that the types work as expected for 2.9 and 3.0, but don't behave correctly for 3.1+. Maybe we should make a bug fix for that in 4.x?

The new typings for TS 4.1+ do actually work for this tested scenario, so the test is brought back for TS 4.1+. If you're alright with it, we can just make this PR only do the type upgrade to TS 3.9+ with no breaking changes (so we can keep this as reselect v4)? I am still planning to make that other PR that will be for the 5.0 release w/ TS 4.1+ typings.

Of course, both of these would require the work done in #483 to use the new CI changes. Let me know if you want me to repurpose this PR or create another one for the "reselect v4 type updates (TS 3.9+)".

markerikson · 2021-02-08T03:09:20Z

Hah, I'm kinda getting lost on what's where now :)

Yeah, let's try things in this order:

Turn off the couple failing tests in the GH Actions PR, as the failing tests are not due to the GH Actions switch, and get that merged in
Update any other dependencies and build tooling
Figure out if there's a reasonable fix to the types that could be made in a 4.x release
Hold off on a larger rewrite of the types, because this will be affected by any noticeable changes to the API that might be involved in a v5

markerikson · 2021-02-08T03:39:38Z

@eXamadeus: so, it turns out that this bug was already reported by @nickmccurdy two years ago :)

#420

nickmccurdy · 2021-02-08T11:06:24Z

types/test.ts

+declare function connect<S, R, P>(selector: Selector<S, R, P[]>):
+  (component: Component<R & S & ([P] extends [never] ? {} : P)>) => Component<P>;


I noticed that R and P are swapped here. Would that impact back compatibility (positively or negatively)?

nickmccurdy · 2021-02-08T11:07:27Z

types/tsconfig.json

+{
+  "compilerOptions": {
+    "module": "commonjs",
+    "lib": ["es6"],
+    "noImplicitAny": true,
+    "noImplicitThis": true,
+    "strictNullChecks": true,
+    "strictFunctionTypes": true,
+    "noEmit": true,
+
+    "baseUrl": ".",
+    "paths": { "src": ["."] }
+  }
+}


Can we use "strict": true or would it be too noisy?

Yeah, I think that would be fine. This was the recommended tsconfig for the old testing library (typings-tester?) I didn't think about updating it, but using strict is definitely worth it IMO.

I'll make sure that's added to whatever final PR goes in. Thanks for the catch!

nickmccurdy · 2021-02-08T11:23:31Z

Whoops sorry, I reviewed this without reading the discussion first. I'm now following the repo so if there are new PRs I should be able to review them. Let me know if I can help with anything else.

eXamadeus · 2021-02-08T14:11:22Z

Hey @nickmccurdy this is still the best one to review the type changes I suppose. But @markerikson brings up a good point that these type changes need to wait until a v5 release.

Here's a list of things I'm planning on doing now:

(Modernize CI setup #482, Improve TypeScript for v4.x #485) Turn off the couple failing tests in the GH Actions PR (ci: replace TravisCI with GitHub Actions #483), as the failing tests are not due to the GH Actions switch, and get that merged in
(Improve TypeScript for v5.0.0 #484) Retarget (recreate?) this PR to v5.0.0 branch. @markerikson I think these types will be best to get in before the API changes, since they should be:
1. easier to modify
2. shouldn't conflict with the changes suggested in Version 5.0.0 proposal #401
3. might actually help the changes in Version 5.0.0 proposal #401 be easier to implement
(Improve TypeScript for v4.x #485) Figure out any type changes that can be in 4.x and make a PR for those. Ones that seem likely:
1. Use Callable Type Signature for Memoize in createSelectorCreator #465
2. Seeing if I can find a fix for As of TypeScript 3.1.1, types erroneously fall back to arity-agnostic createSelector signature when used with arrays #420

For the deps PRs, I can do those too. Looks like #461 and #381 look like good candidates there. I would think the babel upgrade (#381) should probably go first, then #461 can go on top of that. Would you like me to pull those together in a deps PR as well? Or is that something you guys want to do?

markerikson · 2021-02-08T14:16:29Z

@eXamadeus Yep, please go ahead and do a deps PR as well if you have time.

I really appreciate your effort and contributions here!

eXamadeus · 2021-02-08T14:46:57Z

I'm going to make issues to track the "bullet points" I made earlier and I'll edit the comment to link to them.

markerikson · 2021-02-08T15:07:36Z

FYI, we do already have an "upgrade Babel" PR at #381 , which Andarist just rebased. However, it now conflicts with the removal of the Travis file.

Not sure what the best option is for resolving that atm.

eXamadeus · 2021-02-08T15:12:19Z

FYI, we do already have an "upgrade Babel" PR at #381 , which Andarist just rebased. However, it now conflicts with the removal of the Travis file.

Not sure what the best option is for resolving that atm.

I'll comment on that PR, but he just made the same "changes" we just made to the node versions essentially. It looks like just deleting the .travis.yml file in his PR will resolve the conflict and should just work™.

markerikson · 2021-02-08T15:14:00Z

Good call! Just did that via the web UI and I see the GH actions kicking in now.

eXamadeus · 2021-02-08T16:15:13Z

Whoops, I should have made a branch on my fork and not used master. When I updated my master branch it purged this PR of the code.

The latest commit here for reference is 66fcf90. You can see the changes, as they originally were, @ eXamadeus#3 or #487.

eXamadeus commented Jan 30, 2021

View reviewed changes

phryneas reviewed Jan 30, 2021

View reviewed changes

This was referenced Feb 7, 2021

TypeScript errors with --strictFunctionTypes flag. #391

Closed

[TypeScript] defaultMemoize errors #384

Closed

markerikson mentioned this pull request Feb 7, 2021

Modernize CI setup #482

Closed

eXamadeus mentioned this pull request Feb 8, 2021

ci: replace TravisCI with GitHub Actions #483

Merged

3 tasks

nickmccurdy suggested changes Feb 8, 2021

View reviewed changes

This was referenced Feb 8, 2021

Improve TypeScript for v5.0.0 #484

Closed

Improve TypeScript for v4.x #485

Closed

eXamadeus closed this Feb 8, 2021

eXamadeus force-pushed the master branch from 66fcf90 to d889c32 Compare February 8, 2021 15:26

This was referenced Feb 8, 2021

TypeScript update for 4.x #486

Merged

refactor(typescript): upgrade to typescript 4.1, use latest TS to simplify and generalize types #487

Closed

eXamadeus mentioned this pull request Feb 9, 2021

refactor(typescript): upgrade to typescript 4.1, use latest TS to simplify and generalize types eXamadeus/reselect#3

Draft

		declare function connect<S, R, P>(selector: Selector<S, R, P[]>):
		(component: Component<R & S & ([P] extends [never] ? {} : P)>) => Component<P>;

		@@ -0,0 +1 @@
		../src/index.d.ts

refactor(typescript): upgrade to TS 4.1, simplify and generalize types #480

refactor(typescript): upgrade to TS 4.1, simplify and generalize types #480

Conversation

eXamadeus commented Jan 30, 2021 • edited

EDIT:

What

Changes

CI/Build Chores

codecov bot commented Jan 30, 2021 • edited

Codecov Report

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

nickmccurdy Feb 8, 2021 • edited

Choose a reason for hiding this comment

eXamadeus Feb 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eXamadeus Jan 30, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eXamadeus Jan 30, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markerikson commented Jan 30, 2021

phryneas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eXamadeus Jan 30, 2021 • edited

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

eXamadeus Feb 7, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eXamadeus commented Jan 30, 2021

eXamadeus commented Feb 6, 2021 • edited

markerikson commented Feb 7, 2021

markerikson commented Feb 7, 2021

eXamadeus commented Feb 7, 2021 • edited

markerikson commented Feb 7, 2021

eXamadeus commented Feb 7, 2021

markerikson commented Feb 8, 2021

eXamadeus commented Feb 8, 2021

markerikson commented Feb 8, 2021

markerikson commented Feb 8, 2021

nickmccurdy Feb 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickmccurdy commented Feb 8, 2021 • edited

eXamadeus commented Feb 8, 2021 • edited

markerikson commented Feb 8, 2021

eXamadeus commented Feb 8, 2021

markerikson commented Feb 8, 2021

eXamadeus commented Feb 8, 2021 • edited

markerikson commented Feb 8, 2021

eXamadeus commented Feb 8, 2021 • edited

eXamadeus commented Jan 30, 2021 •

edited

codecov bot commented Jan 30, 2021 •

edited

nickmccurdy Feb 8, 2021 •

edited

eXamadeus Feb 8, 2021 •

edited

eXamadeus Jan 30, 2021 •

edited

eXamadeus Jan 30, 2021 •

edited

eXamadeus Jan 30, 2021 •

edited

eXamadeus Feb 7, 2021 •

edited

eXamadeus commented Feb 6, 2021 •

edited

eXamadeus commented Feb 7, 2021 •

edited

nickmccurdy Feb 8, 2021 •

edited

nickmccurdy commented Feb 8, 2021 •

edited

eXamadeus commented Feb 8, 2021 •

edited

eXamadeus commented Feb 8, 2021 •

edited

eXamadeus commented Feb 8, 2021 •

edited