Bug 1925091 - [ci full] Make Android FxaAccountManager supply comma-separate list of values for service=sync#6877
Bug 1925091 - [ci full] Make Android FxaAccountManager supply comma-separate list of values for service=sync#6877AadityaDhingra wants to merge 4 commits intomozilla:mainfrom
Conversation
jonalmeida
left a comment
jonalmeida
left a comment
There was a problem hiding this comment.
Self note: Leaving my offline feedback here to come back to it later.
| @@ -115,7 +115,7 @@ class FxaClient(inner: FirefoxAccount, persistCallback: PersistCallback?) : Auto | |||
| entrypoint: String, | |||
There was a problem hiding this comment.
We should expose the service param here and use the default value from here since there is this thin wrapper around the rust generated FFI code.
| entrypoint: String, | |
| entrypoint: String, | |
| services: List<String> = listOf("sync"), |
| string begin_oauth_flow([ByRef] sequence<string> scopes, [ByRef] string entrypoint); | ||
|
|
||
|
|
||
| string begin_oauth_flow([ByRef] sequence<string> scopes, [ByRef] string entrypoint, [ByRef] sequence<string> service); |
There was a problem hiding this comment.
We can use default values for the udl so that the iOS tests don't fail if that's a preference: https://mozilla.github.io/uniffi-rs/latest/proc_macro/functions.html#default-values
Alternatively, we can also fix the breaking tests too. :)
There was a problem hiding this comment.
Defaults in UDL are done slightly differently: https://mozilla.github.io/uniffi-rs/latest/udl/functions.html. I think a list of strings should be supported, but I'm not totally sure.
There was a problem hiding this comment.
Yeah, sorry I realized too late that I shared the wrong doc link. 🤦
| string begin_oauth_flow([ByRef] sequence<string> scopes, [ByRef] string entrypoint); | ||
|
|
||
|
|
||
| string begin_oauth_flow([ByRef] sequence<string> scopes, [ByRef] string entrypoint, [ByRef] sequence<string> service); |
There was a problem hiding this comment.
Defaults in UDL are done slightly differently: https://mozilla.github.io/uniffi-rs/latest/udl/functions.html. I think a list of strings should be supported, but I'm not totally sure.
components/fxa-client/src/auth.rs
Outdated
| // Allow both &[String] and &[&str] since UniFFI can't represent `&[&str]` yet, | ||
| scopes: &[T], | ||
| entrypoint: &str, | ||
| service: &[String], |
There was a problem hiding this comment.
This could be &[T] as well, using the same hack as scopes. I think that makes some of the Rust code a bit more idiomatic, but it's not a big deal. Both scopes and service would need to be the same type (either &str or &String) but that seems okay to me.
…st of values for service=sync
…st of values for service=sync
…st of values for service=sync
…st of values for service=sync
3 participants
Pull Request checklist
[ci full]to the PR title.