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

Remove Alamofire as a library dependency #736

Merged
merged 9 commits into from
Feb 26, 2024
Merged

Conversation

crazytonyli
Copy link
Contributor

Description

The apps has been running with useURLSession turned on for a couple of weeks. See wordpress-mobile/WordPress-iOS#22540 and woocommerce/woocommerce-ios#11927. There is no major issue reported. And it's likely we'll fix bugs in the new implementation, instead of reverting back to the old one.

This PR removes Alamofire as a dependency to the library, but it's still used in the unit tests to verify some encoding request encoding implementations, i.e. multipart-form.

Testing Details

Changes are covered by unit tests.


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

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.

🎉

This PR removes Alamofire as a dependency to the library, but it's still used in the unit tests to verify some encoding request encoding implementations, i.e. multipart-form.

I hadn't considered the option of keeping it in the unit tests temporarily. It's a great idea, because it lets us focus on removing Alamofire from the production code faster. 👍

@crazytonyli crazytonyli merged commit 40d6a9f into trunk Feb 26, 2024
9 checks passed
@crazytonyli crazytonyli deleted the tonyli-remove-alamofire branch February 26, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants