-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integrate Gravatar SDK #22921
Integrate Gravatar SDK #22921
Conversation
# Conflicts: # Podfile.lock
* Use GravatarURL instead of Gravatar * Use GravatarURL in ListTableViewCell --------- Co-authored-by: Pinar Olguc <[email protected]>
# Conflicts: # Podfile.lock
Co-authored-by: Pinar Olguc <[email protected]>
…2641) Co-authored-by: Pinar Olguc <[email protected]>
…-fetch-profile-from-gravatar-sdk
Co-authored-by: Pinar Olguc <[email protected]>
* Update podfile Gravatar testspecs * bundle exec pod install --------- Co-authored-by: Pinar Olguc <[email protected]>
We won't use the SDK until we have an open api-based endpoint for it
…ery/use-new-identifier-types
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr22921-50e5dbb | |
Version | 24.6 | |
Bundle ID | org.wordpress.alpha | |
Commit | 50e5dbb | |
App Center Build | WPiOS - One-Offs #9347 |
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr22921-50e5dbb | |
Version | 24.6 | |
Bundle ID | com.jetpack.alpha | |
Commit | 50e5dbb | |
App Center Build | jetpack-installable-builds #8391 |
cc: @alpavanoglu @kean Could you take a look as well? Not sure who else to tag so please feel free to tag others who might be interested. We don't expect you to run the tests(although you are highly welcome if you want to). Just let us know if you have any concerns & comments. Thank you! |
Also, here's the alternative change set that imports Gravatar from WordPressUI: https://github.com/wordpress-mobile/WordPress-iOS/compare/task/gravatar-integration?expand=1 The reason why we didn't pursue that is because WordPressUI has deployment target set to iOS 11 and this is too low for our SDK. Gravatar SDK’s is iOS 15. For us, it’s better not to be bound by WPUI’s target version as we go down the road. Because we’ll add more features in the SDK and we would like to make the most of modern Apple APIs. So it would be easier for us to move fwd with this PR. But let us know if you think this one is better. |
# Conflicts: # Podfile.lock
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.
We talked in Slack. We decided to move fwd with this solution. |
Fixes #22543
WordPressUI PR: wordpress-mobile/WordPressUI-iOS#156
WordPressUI.Gravatar
usage withGravatar.AvatarURL
Gravatar.AvatarService.upload(...)
to upload an avatarGravatar.ImageCaching
.extension UIImageView
to add new alternatives of what’s deprecated in WordPressUI. (All related with downloading an Avatar.)GravatarServiceTests
No new functionality is added. This is just to replace code & refactor.
I'll add some screenshots with avatars to make
dangermattic
happy.To test:
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: