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.

Sign up for GitHub

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

Jump to bottom

Mark IssuingCardPinService @Deprecated & suggest alternative #7723

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Mark IssuingCardPinService @Deprecated & suggest alternative #7723

merged 3 commits into from
Dec 21, 2023

Conversation

vbc-stripe
Copy link
Contributor

@vbc-stripe vbc-stripe requested review from a team as code owners December 12, 2023 22:46
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 12, 2023 •
edited
Loading

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: V1, V2)
NEW: paymentsheet-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   3.6 MiB │   3.6 MiB │  0 B │   7.8 MiB │   7.8 MiB │  0 B 
     arsc │   2.3 MiB │   2.3 MiB │  0 B │   2.3 MiB │   2.3 MiB │  0 B 
 manifest │   5.1 KiB │   5.1 KiB │  0 B │  25.4 KiB │  25.4 KiB │  0 B 
      res │ 922.9 KiB │ 922.9 KiB │  0 B │   1.4 MiB │   1.4 MiB │  0 B 
   native │   2.6 MiB │   2.6 MiB │  0 B │     6 MiB │     6 MiB │  0 B 
    asset │     3 MiB │     3 MiB │  0 B │     3 MiB │     3 MiB │  0 B 
    other │ 205.3 KiB │ 205.3 KiB │  0 B │ 462.1 KiB │ 462.1 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.5 MiB │  12.5 MiB │  0 B │  20.9 MiB │  20.9 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 38795 │ 38795 │ 0 (+0 -0) 
   types │ 12986 │ 12986 │ 0 (+0 -0) 
 classes │ 10973 │ 10973 │ 0 (+0 -0) 
 methods │ 53934 │ 53934 │ 0 (+0 -0) 
  fields │ 35892 │ 35892 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  292 │  292 │  0   
 entries │ 6939 │ 6939 │  0

@vbc-stripe vbc-stripe requested review from tillh-stripe and removed request for carlosmuvi-stripe December 12, 2023 23:05
tillh-stripe
tillh-stripe reviewed Dec 14, 2023
View reviewed changes
payments-core/src/main/java/com/stripe/android/IssuingCardPinService.kt
@@ -19,6 +19,7 @@ import kotlin.coroutines.CoroutineContext
/**
* Methods for retrieval / update of a Stripe Issuing card
*/
@Deprecated("Please use Issuing Elements instead: https://stripe.com/docs/issuing/elements")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we link to https://stripe.com/docs/issuing/elements#issuing-elements-and-native-applications instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though this section is immediately relevant to native apps, I think it's better to link to the top of that same doc, as its likely the user won't even know what Issuing Elements is or how it's related to PINs.

Additionally, we've been discussing the possibility of building native Issuing Elements, and if/when that happens, it'll be documented at https://stripe.com/docs/issuing/elements and https://stripe.com/docs/issuing/elements#issuing-elements-and-native-applications will likely be removed

payments-core/src/main/java/com/stripe/android/IssuingCardPinService.kt
@@ -361,6 +363,7 @@ class IssuingCardPinService @VisibleForTesting internal constructor(
stripeAccountId: String? = null,
keyProvider: EphemeralKeyProvider
): IssuingCardPinService {
Log.w(TAG, "Please use Issuing Elements instead: https://stripe.com/docs/issuing/elements")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here: https://stripe.com/docs/issuing/elements#issuing-elements-and-native-applications

CHANGELOG.md Outdated
@@ -2,6 +2,8 @@

## XX.XX.XX - 2023-XX-XX

* [DEPRECATED][7723](https://github.com/stripe/stripe-android/pull/7723) IssuingCardPinService
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add more context here?

@vbc-stripe
Copy link
Contributor Author

force pushed after manually merging rebase conflicts

@vbc-stripe vbc-stripe requested review from tillh-stripe and removed request for tillh-stripe December 18, 2023 17:58
@vbc-stripe vbc-stripe requested review from a team and removed request for tillh-stripe December 21, 2023 21:03
@vbc-stripe vbc-stripe requested review from awush-stripe and removed request for a team December 21, 2023 21:06
@vbc-stripe
Copy link
Contributor Author

@awush-stripe , i think i've addressed all of @tillh-stripe's comments but they won't be back from PTO for a while, so will you please have a look?

awush-stripe
awush-stripe approved these changes Dec 21, 2023
View reviewed changes
Copy link
Collaborator

@awush-stripe awush-stripe left a comment

Choose a reason for hiding this comment

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

lgtm!

@vbc-stripe vbc-stripe merged commit 87c612b into master Dec 21, 2023
11 checks passed
@vbc-stripe vbc-stripe deleted the vbc/deprecate-pin branch December 21, 2023 21:17
@vbc-stripe
Copy link
Contributor Author

lgtm!

thanks for the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tillh-stripe tillh-stripe tillh-stripe left review comments

@awush-stripe awush-stripe awush-stripe approved these changes

Assignees

@awush-stripe awush-stripe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vbc-stripe @awush-stripe @tillh-stripe