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

Re-implement RemoteReaderPost in Swift - Part 1 #561

Open
wants to merge 9 commits into
base: translate-objc-to-swift
Choose a base branch
from

Conversation

crazytonyli
Copy link
Contributor

Description

RemoteReaderPost has lots of parsing functions, it would be very hard to review and easy to make mistakes if translating the ObjC code to Swift in one go. I will break down the translation into a few parts. The first PR here translates the simpliest parsing functions, which are all covered by unit tests.

Testing Details

We should be good if the CI jobs are green.

  • Please check here if your pull request includes additional test coverage.
  • I have considered updating the version in the .podspec file.

@crazytonyli crazytonyli requested a review from a team December 14, 2022 00:22
@crazytonyli crazytonyli self-assigned this Dec 14, 2022
@@ -0,0 +1,17 @@
import Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

This import might be unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's true for the current project setup. But I think it's needed for SPM support.

@crazytonyli crazytonyli changed the base branch from trunk to translate-objc-to-swift December 15, 2022 20:56
@crazytonyli
Copy link
Contributor Author

I've changed the base branch to the dedicated Swift migration branch, see #562 for more context.

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