fix(appservice): Remove erroneous ? operator

[agraven]

Fixes the issue caused by merging #747

[codecov]

Codecov Report

Merging #764 (de04aba) into main (e8b2655) will increase coverage by 5.77%.
The diff coverage is 90.34%.

@@            Coverage Diff             @@
##             main     #764      +/-   ##
==========================================
+ Coverage   71.59%   77.37%   +5.77%     
==========================================
  Files         117       94      -23     
  Lines       16474    13735    -2739     
==========================================
- Hits        11795    10627    -1168     
+ Misses       4679     3108    -1571     

Impacted Files	Coverage Δ
crates/matrix-sdk-base/src/store/ambiguity_map.rs	82.35% <ø> (ø)
crates/matrix-sdk-crypto/src/error.rs	0.00% <ø> (ø)
crates/matrix-sdk-crypto/src/olm/account.rs	82.29% <ø> (+0.27%)	⬆️
crates/matrix-sdk-crypto/src/olm/mod.rs	97.24% <ø> (ø)
...es/matrix-sdk-crypto/src/olm/signing/pk_signing.rs	77.88% <ø> (-16.46%)	⬇️
crates/matrix-sdk/src/client/mod.rs	81.63% <65.21%> (+0.03%)	⬆️
crates/matrix-sdk/src/http_client.rs	88.75% <69.23%> (+0.65%)	⬆️
crates/matrix-sdk-appservice/src/lib.rs	71.36% <80.00%> (+5.67%)	⬆️
crates/matrix-sdk-crypto/src/identities/user.rs	70.54% <85.71%> (+2.73%)	⬆️
crates/matrix-sdk-appservice/tests/tests.rs	96.44% <86.36%> (-1.51%)	⬇️
... and 78 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5243091...de04aba. Read the comment docs.

[Hywan]

👍

[johannescpk]

I'm curious, how did that slip through CI?

[jplatte]

@johannescpk #747 was not up to date with main at the time of merging, where a function had been made non-fallible (and the PR introduced a new call to that function with ? after it).

GitHub has a setting that requires PRs to be up-to-date before merging, but I would argue that's overkill as cases like this are very rare and that setting require manual intervention on a daily basis. There's other solutions such as CI systems that maintain a merge queue where things go through another level of testing before being integrated into the main branch (see for instance Zuul, which I've wanted to try out for Ruma), but they're not as readily available as GitHub actions.

[MTRNord]

GitHub has a setting that requires PRs to be up-to-date before merging, but I would argue that's overkill as cases like this are very rare and that setting require manual intervention on a daily basis. There's other solutions such as CI systems that maintain a merge queue where things go through another level of testing before being integrated into the main branch (see for instance Zuul, which I've wanted to try out for Ruma), but they're not as readily available as GitHub actions.

Good old bors I think is also something which could do this. Iirc rust used this for a while.

[johannescpk]

@jplatte Ah, understood, thanks for the explanation. That's why 5243091 s CI failed after merging - so it became instantly obvious.