-
Notifications
You must be signed in to change notification settings - Fork 252
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
sdk: move get_profile
from Client
to Account
#3238
Conversation
Also rename existing `Account::get_profile` to `Account::get_current_user_profile` and reuse the new function.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3238 +/- ##
=======================================
Coverage 83.63% 83.63%
=======================================
Files 236 236
Lines 24419 24419
=======================================
Hits 20423 20423
Misses 3996 3996 ☔ 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.
Looks good - please can you document the API changes in the changelog?
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.
Looks good, though we'd like some alternative names.
crates/matrix-sdk/src/account.rs
Outdated
/// println!( | ||
/// "You are '{:?}' with avatar '{:?}'", | ||
/// profile.displayname, profile.avatar_url | ||
/// ); | ||
/// # anyhow::Ok(()) }; | ||
/// ``` | ||
pub async fn get_profile(&self) -> Result<get_profile::v3::Response> { | ||
pub async fn get_current_user_profile(&self) -> Result<get_profile::v3::Response> { |
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.
I talked to some folks in the SDK team.
We agreed that we'd prefer this to be called: fetch_user_profile()
.
crates/matrix-sdk/src/account.rs
Outdated
/// # Arguments | ||
/// | ||
/// * `user_id` the matrix id this function downloads the profile for | ||
pub async fn get_profile(&self, user_id: &UserId) -> Result<get_profile::v3::Response> { |
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.
And this one should be called fetch_user_profile_of()
.
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.
Looks good, please just add a changelog entry like @andybalaam requested.
Also rename existing
Account::get_profile
toAccount::get_current_user_profile
and reuse the new function.Signed-off-by: