-
Notifications
You must be signed in to change notification settings - Fork 999
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
add detachSourceFromCustomer and updateCustomer to BackendAPIAdapter #813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should remove deprecation flags and also looks like there's a logic error in source detach handling.
Instead of providing your own backend API adapter, you can now create an | ||
`STPCustomerContext`, which will manage retrieving and updating a | ||
Stripe customer for you. @see STPCustomerContext.h | ||
|
||
*/ | ||
__attribute__((deprecated)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unmark this as deprecated?
your own backend instead of using `STPCustomerContext`, you should make your | ||
application's API client conform to this interface. It provides a "bridge" from | ||
the prebuilt UI we expose (such as `STPPaymentMethodsViewController`) to your | ||
backend to fetch the information it needs to power those views. | ||
|
||
@deprecated Use `STPCustomerContext`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unmark as deprecated?
// Cannot detach payment methods without customer context | ||
return NO; | ||
} | ||
|
||
if (![self.apiAdapter respondsToSelector:@selector(detachSourceFromCustomer:completion:)]) { | ||
// Cannot detach payment methods if customerContext is an apiAdapter | ||
// that doesn't implement detachSource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this return NO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh derp, good catch
#pragma clang diagnostic push | ||
#pragma clang diagnostic ignored "-Wdeprecated" | ||
@property (nonatomic, strong, nullable, readwrite) id<STPBackendAPIAdapter> apiAdapter; | ||
#pragma clang diagnostic pop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should unmark as deprecated and remove these pragmas
@bdorfman-stripe re-r? I've undeprecated and updated docs in a7e31aa The documentation updates are a bit noisy - mostly fixing line breaks. I made some actual content changes in the STPBackendAPIAdapter docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
… of frame count (#813) This change removes the frame minimum from the MotionBlurDetector in place of a time minimum for consecutive frames. This is more consistent because different devices have different frame rates, depending on their CPU or whether they contain a neural engine. Previous to this change, we were still seeing some motion blur in images on faster performing devices. The current default is 500ms, however this will eventually be server driven.
r? @bdorfman-stripe
Requested in #812 .
I decided to add these methods to
STPBackendAPIAdapter
, to keepSTPCustomerContext.h
clean (most users shouldn't need to know these exist). It also lets users that want to stick withSTPBackendAPIAdapter
to add deletion and shipping update functionality.