graphile / migrate

Opinionated SQL-powered productive roll-forward migration tool for PostgreSQL.
MIT License
751 stars 58 forks source link

chore: cleanup a few lint errors #150

Closed maxkorp closed 2 years ago

maxkorp commented 2 years ago

Description

Fixes some any uses and some deprecations.

uses of any should be pretty self explanatory.

There are also a few deprecated method calls (substr vs substring and Assert.equals vs Assert.strictEquals. I checked files manually for deprecations in vscode. I tried adding a lint rule (https://github.com/gund/eslint-plugin-deprecation) but it doesn't seem to catch everything (didn't catch the substr for example). Added a pretty big install change so I left it off.

Performance impact

None

Security impact

None. The only change to the JS output is assert.strictEqual vs assert.equal in current.js

Checklist

maxkorp commented 2 years ago

Ok pushed up anything. Since the resolved convos get hidden I believe, I'll relay them here

  1. Is _gmlogged a migrate thing? Can we (and do we) want to change it to _gmLogged?

  2. In run in run.ts, we use withQuery (from pgReal), and pass in runQueryWithErrorInstrumentation. Is there something wrong that the T type from runQueryWithErrorInstrumentation doesn't get bubbled up to withQuery and there by run? Or is that expected in typescript?

Thanks for the help here. My typescript fu is super weak; since I'm used to either wild west vanilla JS or extremely strict/rigid ocaml, I don't really know where to lay my expectations of behaviors with typescripts in-betweeny level of strictness. Just trying to clean up the assault I get when running tests etc.

benjie commented 2 years ago

(I also commented in the collapsed discussions.)