Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies, require node 20 #775

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

tadzik
Copy link
Collaborator

@tadzik tadzik commented Mar 26, 2024

No description provided.

@tadzik tadzik requested a review from a team as a code owner March 26, 2024 12:08
@tadzik tadzik marked this pull request as draft March 26, 2024 12:11
@tadzik tadzik marked this pull request as ready for review March 29, 2024 09:32
@Cadair
Copy link
Collaborator

Cadair commented Apr 4, 2024

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.

@tadzik
Copy link
Collaborator Author

tadzik commented Apr 5, 2024

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.

Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's needed to make that test pass?

@@ -38,7 +38,7 @@ jobs:
runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should make sure this works before merging.

@@ -25,14 +25,6 @@ const log = new Logger("substitutions");
const ATTACHMENT_TYPES = ["m.audio", "m.video", "m.file", "m.image"];
const PILL_REGEX = /<a href="https:\/\/matrix\.to\/#\/(#|@|\+)([^"]+)">([^<]+)<\/a>/g;

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is now part of the functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants