Update dependencies, require node 20 #775
Conversation
tadzik
force-pushed
the
tadzik/dep-updates
branch
from
ec8ee33 to
b68e76c
Compare
tadzik
force-pushed
the
tadzik/dep-updates
branch
from
b68e76c to
f280497
Compare
|
Looks sane to me, but the postgres integration test fail seems legit? edit: oh wait, looks like it's just busted, so probably not related to this PR. |
|
It's worth considering updating our matrix-appservice-bridge here and dropping NeDB while at it – we'd need to support a DB migration though. |
| - run: yarn test:integration | ||
|
|
||
| integration-postgres: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Ideally we should make sure this works before merging.
| const ATTACHMENT_TYPES = ["m.audio", "m.video", "m.file", "m.image"]; | ||
| const PILL_REGEX = /<a href="https:\/\/matrix\.to\/#\/(#|@|\+)([^"]+)">([^<]+)<\/a>/g; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Assuming this is now part of the functionality.
There was a problem hiding this comment.
Correct. I've now added tests to verify this.
It needed the default timeout increased, 2 seconds was not enough to apply DB migrations. Should be good now. |
changelog.d/775.misc
Outdated
| @@ -0,0 +1 @@ | |||
| Require node 20, update dependencies. | |||
There was a problem hiding this comment.
This needs to be a .removal, as we're removing support. I'd put in stronger wording to say that Node 16 and 18 are no longer supported, and users need to update to Node 20.
I wouldn't both stating that dependencies are updated, it doesn't help the user any to know that.
There was a problem hiding this comment.
Good points. Changed in 7f3cdec
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js |
There was a problem hiding this comment.
N.B. Remember to update the Dockerfile.
3 participants
Signed off by: Tadeusz „tadzik” Sośnierz <signoff at tadzik dot net>