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 unneeded APIRequest conformance from Demo #178

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

scannillo
Copy link
Collaborator

Reason for changes

Make it clear that the Demo does not rely on the SDK's internal APIRequest protocol.

We're also planning to remove this protocol entirely (👀 #177)

Summary of changes

  • Remove unneeded conformance to APIRequest on ClientIDRequest in demo app

Checklist

  • Added a changelog entry

Authors

@scannillo

@scannillo scannillo requested a review from a team as a code owner August 9, 2023 15:28
@scannillo scannillo mentioned this pull request Aug 9, 2023
1 task
@@ -1,7 +1,7 @@
import Foundation
import CorePayments

struct ClientIDRequest: APIRequest {
struct ClientIDRequest {

Copy link
Collaborator

@KunJeongPark KunJeongPark Aug 9, 2023

Choose a reason for hiding this comment

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

Sammy, on my draft PR for vault without purchase, (

private func createUrlRequest(
) I had reused this createUrlRequest for setup token call with APIRequest protocol. I have to have some other protocol so we can take in different request structs to create urlRequest for demo app.

What I can do right now is just create a separate createUrlRequest functions for both REST calls

Copy link
Collaborator

@KunJeongPark KunJeongPark Aug 9, 2023

Choose a reason for hiding this comment

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

I addressed it in latest commit in the PR. It's redundant but I will consolidate if we have another protocol.

Copy link
Collaborator Author

@scannillo scannillo Aug 9, 2023

Choose a reason for hiding this comment

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

The Demo should never have been using APIRequest. Awesome, I'll check out your new updates!

@scannillo scannillo merged commit 0010733 into main Aug 10, 2023
@scannillo scannillo deleted the remove-apirequest-demo branch August 10, 2023 14:57
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