experiment - using spm for demo app#2565
Conversation
There was a problem hiding this comment.
Code Review
This pull request primarily transitions several APIs and types from package or internal access levels to public across multiple modules (such as Adyen, AdyenActions, AdyenCard, and AdyenSession). It also consolidates and refactors the Swift Package Manager (SPM) and non-SPM bundle resource extensions into unified files using conditional compilation (#if SWIFT_PACKAGE). The review feedback recommends marking helper properties like coreBundle, actions, and cardBundle as private within their respective bundle extensions to prevent polluting the Bundle namespace.
| /// This will be available when using swift packages, open the `Package.swift` file and see. | ||
| package static let coreInternalResources: Bundle = .module | ||
| #else | ||
| static let coreBundle: Bundle = .init(for: AdyenContext.self) |
There was a problem hiding this comment.
The coreBundle property is only used within this extension to resolve coreInternalResources. It should be marked as private to avoid polluting the Bundle namespace package-wide.
| static let coreBundle: Bundle = .init(for: AdyenContext.self) | |
| private static let coreBundle: Bundle = .init(for: AdyenContext.self) |
| /// This will be available when using swift packages, open the `Package.swift` file and see. | ||
| static let actionsInternalResources: Bundle = .module | ||
| #else | ||
| static let actions: Bundle = .init(for: RedirectComponent.self) |
There was a problem hiding this comment.
The actions property is only used within this extension to resolve actionsInternalResources. It should be marked as private to avoid polluting the Bundle namespace within the AdyenActions module.
| static let actions: Bundle = .init(for: RedirectComponent.self) | |
| private static let actions: Bundle = .init(for: RedirectComponent.self) |
| /// This will be available when using swift packages, open the `Package.swift` file and see. | ||
| static let cardInternalResources: Bundle = .module | ||
| #else | ||
| static let cardBundle: Bundle = .init(for: CardComponent.self) |
There was a problem hiding this comment.
The cardBundle property was previously private in CardBundleExtension.swift. Since it is only used internally within this extension to resolve cardInternalResources, it should be marked as private to prevent polluting the Bundle namespace within the AdyenCard module.
| static let cardBundle: Bundle = .init(for: CardComponent.self) | |
| private static let cardBundle: Bundle = .init(for: CardComponent.self) |
|
ℹ️ No baseline data found for 'develop'.
|
👀 26 public changes detectedComparing
|
Just challenging te status quo, why do we need to have the framework targets in Adyen.xcodeproj at all?
Why can't the test app directly use the SPM Package.swift (like an sdk integrator).
Would there any concerns to this approach?
Consideratins:
We need to re-look at the library evolution test. The purpose of it - as it currently builds dropin framework from the Adyen.xcodeproj. Not sure why.
The demo app cannot directly access many things which were
packageearlier. Which i feel is already a good thing to know. In this PR i've made them public, but they should remain package and we should find another way to have it in the demo app.This MR should not be merged, as it is just an experiment to prove it is possible and check if this setup is developer friendly.