-
Notifications
You must be signed in to change notification settings - Fork 7
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
[PM-13166] Rename Client* to *Client #27
Conversation
No New Or Fixed Issues Found |
a3cbf02
to
d1024de
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #27 +/- ##
==========================================
+ Coverage 64.43% 64.45% +0.02%
==========================================
Files 188 188
Lines 13832 13832
==========================================
+ Hits 8912 8915 +3
+ Misses 4920 4917 -3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks for taking this on!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a breaking change for our clients, would be good to make the mobile team aware of this so they can update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch by Oscar! Make sure to let Oscar know when this has been merged so that he can publish the Swift lib.
Btw @tangowithfoxtrot did you test opening and building the test swift project in crates/bitwarden-uniffi/swift/iOS/App.xcodeproj
?
We also have the android project ... |
@Hinton and @coroiu, I was able to build and run the Android project without needing to change anything in |
The project's we're referencing exists in this repository under https://github.com/bitwarden/sdk-internal/tree/main/crates/bitwarden-uniffi. |
Android seems fine, but the iOS sample has some build errors. (You should update the branch first though since there are 2 errors that are fixed on main now). |
Alright. I merged the updates from main and was able to build the example Swift and Kotlin applications successfully. The Swift app required updating the symbol names, while the Kotlin app did not. |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-13166
📔 Objective
Use more intuitive naming by updating
Client<_>
to<_>Client
.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes