Skip to content
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

Update Alamofire to v5 #22707

Merged
merged 9 commits into from
Feb 28, 2024
Merged

Update Alamofire to v5 #22707

merged 9 commits into from
Feb 28, 2024

Conversation

crazytonyli
Copy link
Contributor

@crazytonyli crazytonyli commented Feb 26, 2024

Changes include:

  • Updating WordPressKit to the trunk revision, which does not depend on Alamofire.
  • Updating Alamofire, AlamofireImage, and AlamofireNetworkActivityIndicator to the latest versions.
  • Replacing deprecated Alamofire API usages with new APIs.

I don't think we need AlamofireNetworkActivityIndicator, considering UIApplication.networkActivityIndicatorVisible has been deprecated and iOS doesn't even show a spinner in status bar any more (at least I can't find a way to make it).

Test Instructions

You can test AlamofireImage changes by verifying images in Reader and Posts list can be displayed.

Regression Notes

  1. Potential unintended areas of impact
    None.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    What are described in the "Test Instructions" section.

  3. What automated tests I added (or what prevented me from doing so)
    None.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist: N/A

@crazytonyli crazytonyli added this to the 24.4 milestone Feb 26, 2024
@crazytonyli crazytonyli self-assigned this Feb 26, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 26, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22707-ca871d8
Version24.3
Bundle IDcom.jetpack.alpha
Commitca871d8
App Center Buildjetpack-installable-builds #8027
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 26, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22707-ca871d8
Version24.3
Bundle IDorg.wordpress.alpha
Commitca871d8
App Center BuildWPiOS - One-Offs #9004
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Jetpack with the reader and by checking that the https://giotestdotsite.wordpress.com/2024/02/27/tenor-post-with-alamofire-5/

@mokagio
Copy link
Contributor

mokagio commented Feb 27, 2024

I don't think we need AlamofireNetworkActivityIndicator, considering UIApplication.networkActivityIndicatorVisible has been deprecated and iOS doesn't even show a spinner in status bar any more (at least I can't find a way to make it).

+1

This Stack Overflow answer is useful in that it says "The deprecation message says: Provide a custom network activity UI in your app if desired."

Also, AlamofireNetworkActivityIndicator hasn't been updated since April 2020. I think it's safe to remove it in a dedicated PR. We can always revert if something goes wrong.

@crazytonyli crazytonyli merged commit 1393161 into trunk Feb 28, 2024
23 checks passed
@crazytonyli crazytonyli deleted the alamofire-changes-in-wordpresskit branch February 28, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants