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

Deprecation Lines for BTPayPalNativeCheckout Module #1341

Merged
merged 8 commits into from
Jul 2, 2024

Conversation

KunJeongPark
Copy link
Contributor

@KunJeongPark KunJeongPark commented Jun 20, 2024

Summary of changes

  • Add deprecation message -
    @available(*, deprecated, message: "BraintreePayPalNativeCheckout Module is deprecated, use BraintreePayPal Module instead") for all public classes and enums for BraintreePayPalNativeCheckout module

Checklist

  • Added a changelog entry

Authors

List GitHub usernames for everyone who contributed to this pull request.

@KunJeongPark KunJeongPark requested a review from a team as a code owner June 20, 2024 23:18
@KunJeongPark KunJeongPark changed the title Deprecation Lines for BTPayPalNativeCheckoutClient Deprecation Lines for BTPayPalNativeCheckout Module Jun 20, 2024
@KunJeongPark KunJeongPark force-pushed the BTPayPalNativeCheckoutDeprecateMessage branch from 37fdd07 to 1a51dd5 Compare June 21, 2024 14:09
CHANGELOG.md Outdated
Comment on lines 6 to 7
* BraintreePayPalNativeCheckout
* Add deprecated warning in public classes and methods
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* BraintreePayPalNativeCheckout
* Add deprecated warning in public classes and methods
* BraintreePayPalNativeCheckout (DEPRECATED)
* **Note:** This module is deprecated and will be removed in a future version of the SDK
* Add deprecated warning message to all public classes and methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here: 59485d6

@@ -2,6 +2,7 @@ import Foundation
import UIKit
import BraintreePayPalNativeCheckout

@available(*, deprecated, message: "BraintreePayPalNativeCheckout Module is deprecated, use BraintreePayPal Module instead")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is our demo app, we don't need a deprecation message here, just in the public classes

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, but I got lint errors from referencing MXO classes here

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have a linter on Braintree. Do you mean Cocoapods pod lib lint?

Copy link
Contributor Author

@KunJeongPark KunJeongPark Jun 21, 2024

Choose a reason for hiding this comment

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

yes. I think so. I saw a cocoapods test failure.

Copy link
Contributor Author

@KunJeongPark KunJeongPark Jun 21, 2024

Choose a reason for hiding this comment

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

https://github.com/braintree/braintree_ios/actions/runs/9614991255/job/26521134948
Oh maybe it was just the other non public classes and protocols that was causing the error.
I know on PPCP, I had failure without annotation on demo app files, so I conflated the two things.
I'll delete that line, I do see the lint warnings locally without that line there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks ok without the line there! Are we ok with the lint warning here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed here: 991982b

@@ -37,6 +39,7 @@ import PayPalCheckout
self.nativeCheckoutProvider = nativeCheckoutProvider
}

@available(*, deprecated, message: "BraintreePayPalNativeCheckout Module is deprecated, use BraintreePayPal Module instead")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move this one above the docstring instead of above the mark

Copy link
Contributor Author

@KunJeongPark KunJeongPark Jul 1, 2024

Choose a reason for hiding this comment

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

Great catch! Addressed here: 6e51baf

Copy link
Contributor

@agedd agedd left a comment

Choose a reason for hiding this comment

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

lgtm!

@KunJeongPark KunJeongPark merged commit 80e324c into main Jul 2, 2024
6 of 7 checks passed
@KunJeongPark KunJeongPark deleted the BTPayPalNativeCheckoutDeprecateMessage branch July 2, 2024 14:25
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.

4 participants