Skip to content

integrations: Enhanced matrix bridge. (Fix #723) #765

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

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

ro-i
Copy link
Contributor

@ro-i ro-i commented Aug 25, 2022

Hi :) This PR aims to fix #723 for Windows.
As discussed there, python-magic seems to be problematic on Windows. However, I noticed in the output of the CI that python-magic-bin doesn't get installed as requested. (It's needed to provide the Windows library support as described here). Thus, my first attempt to fix the Windows support is trying to get python-magic-bin properly installed. If this doesn't work, I disable the magic-support in Windows completely.

@ro-i
Copy link
Contributor Author

ro-i commented Aug 25, 2022

Ok, this does not seem to work. Unfortunately without useful error message in the logs :/
So I'll completely disable python-magic on Windows...

@ro-i ro-i force-pushed the matrix-multiple-bridges-fix-windows branch from dbccca2 to 37d8fa3 Compare August 26, 2022 09:26
@ro-i
Copy link
Contributor Author

ro-i commented Aug 26, 2022

The Windows tests now succeed, just the code coverage is not yet happy...

@rht
Copy link
Contributor

rht commented Sep 2, 2022

Can you highlight the change you made that makes it work on Windows? I couldn't spot any difference. The requirements.txt looks the same.

@rht
Copy link
Contributor

rht commented Sep 2, 2022

Strange. The coverage should have been identical to 12b5e7a.

@ro-i ro-i force-pushed the matrix-multiple-bridges-fix-windows branch from 37d8fa3 to e98d185 Compare September 8, 2022 09:16
@ro-i ro-i force-pushed the matrix-multiple-bridges-fix-windows branch from e98d185 to daacae6 Compare September 8, 2022 10:24
@timabbott timabbott merged commit 63c259b into zulip:main Sep 8, 2022
@timabbott
Copy link
Member

Merged, thanks @ro-i!

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

Successfully merging this pull request may close these issues.

4 participants