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

Rebase/element ios 1.10.2 #761

Merged
merged 355 commits into from
Feb 22, 2023
Merged

Rebase/element ios 1.10.2 #761

merged 355 commits into from
Feb 22, 2023

Conversation

NicolasBuquet
Copy link
Contributor

@NicolasBuquet NicolasBuquet commented Feb 22, 2023

alfogrillo and others added 30 commits January 30, 2023 19:15
Inform the user about decryption errors during a voice broadcast
…ad_option_for_rooms

Flescio/7253 add mar kas unread option for rooms
…support

Labs: Rich text editor: enable list items indentation
@@ -406,8 +406,7 @@ final class BuildSettings: NSObject {

// MARK: - Polls

static let pollsEnabled = false // Currently disabled in Tchap.
static var pollsHistoryEnabled: Bool = false
static let pollsEnabled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

@NicolasBuquet did you enable the polls on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I missed it. I fixed it.

sdkOptions.enableCryptoSDK = isEnabled
sdkOptions.enableStartupProgress = isEnabled
} else {
MXLog.debug("[CommonConfiguration] Crypto SDK is not available)")
Copy link
Contributor

Choose a reason for hiding this comment

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

@NicolasBuquet FYI this configuration is related to the new Rust based Crypto SDK.
enableCryptoSDK should stay false until you decide to switch on this new Crypto SDK. You're using the previous one by default. Let us know if you need more details on this new Crypto SDK

You may have a look on sdkOptions.isCryptoSDKAvailable. This flag is true by default now at the matrix-ios-sdk level (https://github.com/matrix-org/matrix-ios-sdk/blob/develop/MatrixSDK/MXSDKOptions.m#L58). You should be able to keep the control with enableCryptoSDK flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I updated the PR with your advice.

Copy link
Contributor

@yostyle yostyle left a comment

Choose a reason for hiding this comment

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

LGTM

let isEnabled = RiotSettings.shared.enableCryptoSDK
// Tchap : force to not use the new Rust based Crypto SDK
// let isEnabled = RiotSettings.shared.enableCryptoSDK
let isEnabled = false
Copy link
Contributor

Choose a reason for hiding this comment

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

The value is false by default. Why do you need to force this value ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@NicolasBuquet my comment on this code was a remark. You should keep the code from Element here
This is important to track RiotSettings.shared.enableCryptoSDK in your next rebase (to detect when it would be true by default). Currently it is false by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted the commit.
I saw the default value was false. But I didn't know if this property was changed to true by any way in another place of the sdk. So, to be sure, I preferred set it to false when using it in Tchap.

@yostyle yostyle self-requested a review February 22, 2023 12:50
@yostyle
Copy link
Contributor

yostyle commented Feb 22, 2023

@NicolasBuquet you have a conflict not resolved.

@NicolasBuquet NicolasBuquet force-pushed the rebase/element-ios-1.10.2 branch from e8cf398 to 84bb705 Compare February 22, 2023 13:40
@NicolasBuquet NicolasBuquet merged commit 56722cc into develop Feb 22, 2023
@NicolasBuquet NicolasBuquet deleted the rebase/element-ios-1.10.2 branch February 22, 2023 14:25
@NicolasBuquet NicolasBuquet linked an issue Feb 23, 2023 that may be closed by this pull request
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.

Rebase against Element v1.10.2