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

VEX-5682: iOS Swift Conversion #11

Merged
merged 15 commits into from
Oct 27, 2021
Merged

Conversation

nickfujita
Copy link

@nickfujita nickfujita commented Oct 18, 2021

Converts iOS implementation from objc to swift

Jira: VEX-5682
https://jira.tenkasu.net/browse/VEX-5682

Velocity PR: https://github.com/crunchyroll/velocity/pull/1966

@nickfujita
Copy link
Author

nickfujita commented Oct 22, 2021

@cristi-lupu There is probably more cleanup and improvement that can be done, but going to cap off the work here in order to move forward on the project. All previous functionality should be working the same.

Would you mind having a look over the Swift code, and provide any feedback on improvements that can be made to the syntax or patterns used?

let certificateUrl: String?
let base64Certificate: Bool?

let json: NSDictionary?

Choose a reason for hiding this comment

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

Is there any reason you want to keep the initial dictionary object? Applicable for other models as well.

Copy link
Author

Choose a reason for hiding this comment

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

since there is logic that sends back the input dictionary back to the client, instead of converting to DRMParams, then converting back to NSDictionary, this keeps a reference to the original to simply send back

@nickfujita
Copy link
Author

@cristi-lupu thanks for reviewing the swift code! updated.

@nickfujita nickfujita merged commit 8b75438 into master Oct 27, 2021
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.

3 participants