Consolidate all bridged engine wrappers into a single wrapper and macro. - #7464
Conversation
Seeing mozilla#7439 fly past reminded me that I was intending to try and remove the BridgedEngine entirely - this doesn't quite achieve that, but does get closer.
jo
left a comment
There was a problem hiding this comment.
Thanks a lot, that's really cool!
What would we need to do to get rid of the $guid type parameter? Do you have any idea why we don't use a sync_guid::Guid for logins as well? Is it because we need to expose it, and UniFFI does not support a sequence<Guid> thing yet?
bendk
left a comment
There was a problem hiding this comment.
The changes look good to me. I still don't totally understand the bridged engine code, but AFAICT this macro code is exactly the same as before and it makes it easier to add a new bridged engine.
IIRC, it predates the guid type. I think uniffi would handle it fine - like places, it would be a "custom type". OTOH though, I'm a little ambivalent about that type, in that it doesn't really add much value - there's no validation that it is shaped like a guid and we have Into/From to move between strings and guids. It is kinda nice as a kind of documentation though, but could also be viewed as extra complexity without much value. (I can't argue with the fact that we are inconsistent with its usage is quite smelly though, so maybe moving logins to guid would make sense - it would be entirely transparent to foreign code as there it really is just a string) |
…ro. (mozilla#7464) Seeing mozilla#7439 fly past reminded me that I was intending to try and remove the BridgedEngine entirely - this doesn't quite achieve that, but does get closer.
Seeing #7439 fly past reminded me that I was intending to try and remove the BridgedEngine entirely - this doesn't quite achieve that, but does get closer.
Pull Request checklist
[ci full]to the PR title.