feat: typescript and migrations #51
Conversation
|
SonarCloud Quality Gate failed.
|
There was a problem hiding this comment.
Super, ça me semble bon !
J'ai laissé quelques notes, c'est plus ou moins ignorable.
Concernant les tests, je ne sais justement pas trop comment faire... Tu as pu tester de ton côté ? Qu'est-ce qui peut nous rassurer quant au fait que tout va toujours bien aller ?
Une deuxième question : pour les tests on a dû ajouter un build pour le TS. C'est bon pour le déploiement, on gère le build aussi ?
| | DESTINATION_TABLE | `matomo` | | ||
| | STARTDATE | default to today() | | ||
| | RESULTPERPAGE | matomo pagination (defaults to 500) | | ||
| | INITIAL_OFFSET | How many days to fetch on initialisation (defaults to 3) | |
There was a problem hiding this comment.
ça veut dire qu'il faut qu'on mette à jour nos env partout ? Si oui, on en parle, si non tu peux ignorer !
There was a problem hiding this comment.
non c'est juste que ce n'était pas documenté
| }); | ||
| //const isoDate = (date) => formatISO(date, { representation: "date" }); | ||
|
|
||
| // jest.mock("pg", () => { |
There was a problem hiding this comment.
Pourquoi on ne teste plus tout ça ? Si c'est inutile on peut supprimer j'imagine, plutôt que de garder un code qu'on ne sait pas pourquoi il est là. Ou alors on pourrait noter pourquoi c'est là mais pas là.
| // return MatomoClient; | ||
| // }); | ||
|
|
||
| test("test", () => {}); |
There was a problem hiding this comment.
Mon préféré.
| test("test", () => {}); |
| // }); | ||
|
|
||
| test("test", () => {}); | ||
| /* |
There was a problem hiding this comment.
En fait plus globalement je ne comprends pas pourquoi ce fichier est commenté. Je suggère de l'enlever ou d'enlever les parties qu'on ne veut plus, quitte à les refaire.
There was a problem hiding this comment.
C'est dommage github montre pas les diff juste un fichier supprimé et un ajouté. Du coup je survole :'(
| if (extension) { | ||
| console.error("pg_partman extension detected; Skip migrations"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
C'est la présence de partman qui conditionne les migrations, pourquoi ?
There was a problem hiding this comment.
c'était dans un souci de retro-compatibilité avec les anciennes DB qui utilisent partman, mais je vais virer
|
remplacé par #54 avec de nouveaux tests |
Add kysely and database mirgations