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

Default to staging SFU for non-production builds #6696

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

awaitlink
Copy link
Contributor

@awaitlink awaitlink commented Dec 2, 2023

Contributor checklist:

  • My contribution is not related to translations.
  • My commits are in nice logical chunks with good commit messages
  • My changes are rebased on the latest main branch
  • A yarn ready run passes successfully (more about tests here)
  • My changes are ready to be shipped to users

Description

Reason for change

Currently, the staging builds (including the official ones for Public Username Testing (Staging Environment)) use the production SFU by default.

The production (or the test) SFU doesn't work (i.e. group calls are cancelled immediately) in the staging environment (because of different ZK parameters, I believe), requiring manually changing the SFU url via SignalDebug.setSfuUrl to the staging SFU.

Change

This PR changes the default SFU for builds to the staging SFU, overriding it with the production SFU only for production builds.

Other clients

With this PR, Desktop's behavior aligns with Signal Android and Signal iOS, both of which use the staging SFU by default in staging builds.

Testing

Successfully tried a group call (video and audio) between primary iOS physical device and the development Desktop for:

  • Staging: Default build without any changes + iOS staging custom build 6.52.0.3.
  • Production: Copying production.json to local-development.json and building + iOS beta 6.52.0.5.

Copy link
Contributor

@indutny-signal indutny-signal left a comment

Choose a reason for hiding this comment

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

LGTM

@indutny-signal
Copy link
Contributor

Thank you! This got merged and the PR will be closed when we do our next release!

@indutny-signal indutny-signal merged commit 7a3d9f6 into signalapp:main Dec 7, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants