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

Refactor: extracted all log strings #2600

Merged
merged 1 commit into from
Jun 9, 2023
Merged

Refactor: extracted all log strings #2600

merged 1 commit into from
Jun 9, 2023

Conversation

NachoSoto
Copy link
Contributor

I'm working on improving our logging setup, which will be made easier if we don't use hardcoded strings in our logs.

@NachoSoto NachoSoto requested a review from a team June 8, 2023 19:11
@@ -3479,7 +3478,6 @@
574A2F4F282D7B9E00150D40 /* PostOfferDecodingTests.swift in Sources */,
2DDF41CE24F6F4C3005BC22D /* InAppPurchaseBuilderTests.swift in Sources */,
573A10DB2800AF4700F976E5 /* PurchaseErrorTests.swift in Sources */,
B300E4BF26D436F900B22262 /* LogIntent.swift in Sources */,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was unnecessarily being compiled for the Unit Tests target as well.


// swiftlint:disable identifier_name

enum DiagnosticsStrings {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started this, which will grow more with the upcoming diagnostics.

enum RestoreStrings {

// swiftlint:disable identifier_name
case restorepurchases_called_with_allow_sharing_appstore_account_false_warning
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only had one string, so I combined it in PurchaseStrings.

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #2600 (8072c7b) into main (ce47044) will decrease coverage by 0.05%.
The diff coverage is 65.45%.

❗ Current head 8072c7b differs from pull request most recent head 777812a. Consider uploading reports for the commit 777812a to get more accurate results

@@            Coverage Diff             @@
##             main    #2600      +/-   ##
==========================================
- Coverage   86.24%   86.19%   -0.05%     
==========================================
  Files         205      205              
  Lines       14328    14360      +32     
==========================================
+ Hits        12357    12378      +21     
- Misses       1971     1982      +11     
Impacted Files Coverage Δ
Sources/Logging/Strings/Strings.swift 100.00% <ø> (ø)
Sources/Purchasing/Configuration.swift 81.00% <0.00%> (ø)
Sources/Purchasing/Offering.swift 92.30% <0.00%> (-1.03%) ⬇️
...ources/Purchasing/StoreKit1/StoreKit1Wrapper.swift 84.96% <0.00%> (+0.55%) ⬆️
...s/Purchasing/Purchases/PurchasesOrchestrator.swift 85.58% <25.00%> (ø)
Sources/Logging/Strings/ConfigureStrings.swift 87.00% <33.33%> (-3.43%) ⬇️
Sources/Logging/Strings/OfferingStrings.swift 93.57% <66.66%> (-0.77%) ⬇️
Sources/Logging/Strings/StoreKitStrings.swift 88.88% <70.00%> (-2.67%) ⬇️
Sources/Logging/Strings/PurchaseStrings.swift 88.73% <85.71%> (-0.11%) ⬇️
Sources/Logging/Strings/DiagnosticsStrings.swift 100.00% <100.00%> (ø)
... and 3 more

... and 2 files with indirect coverage changes

case payment_queue_wrapper_delegate_call_sk1_enabled

// swiftlint:disable:next identifier_name
case restorepurchases_called_with_allow_sharing_appstore_account_false
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick and it was like this before but should this be restore_purchases_called_with_allow_sharing_appstore_account_false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was like this because the method is restorePurchases, so this is one word to signal that.

@NachoSoto NachoSoto merged commit 3388bc5 into main Jun 9, 2023
@NachoSoto NachoSoto deleted the extract-log-strings branch June 9, 2023 16:48
This was referenced Jun 13, 2023
NachoSoto pushed a commit that referenced this pull request Jun 13, 2023
**This is an automatic release.**

### New Features
* New `DebugViewController`: UIKit counterpart for SwiftUI's
`debugRevenueCatOverlay` (#2631) via NachoSoto (@NachoSoto)
* Created `PaywallExtensions`: `StoreView` and `SubscriptionStoreView`
overloads for `Offering` (#2593) via NachoSoto (@NachoSoto)
* Introduced `debugRevenueCatOverlay()`: new SwiftUI debug overlay
(#2567) via NachoSoto (@NachoSoto)
### Bugfixes
* Removed `preventPurchasePopupCallFromTriggeringCacheRefresh`, update
caches on `willEnterForeground` (#2623) via NachoSoto (@NachoSoto)
* Fixed `Catalyst` build with `Xcode 15 beta 1` (#2586) via NachoSoto
(@NachoSoto)
### Dependency Updates
* Bump danger from 9.3.0 to 9.3.1 (#2592) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* `StoreTransaction`: added new `Storefront` to API testers (#2634) via
NachoSoto (@NachoSoto)
* `DebugView`: added snapshot tests (#2630) via NachoSoto (@NachoSoto)
* `verifyNoUnfinishedTransactions`/`verifyUnfinishedTransaction`: added
missing `#file` parameter (#2625) via NachoSoto (@NachoSoto)
* `PostReceiptDataOperation`: clean up cache key (#2628) via NachoSoto
(@NachoSoto)
* `PurchasesOrchestrator`: also get `Storefront` from SK1 (#2629) via
NachoSoto (@NachoSoto)
* `CI`: disable iOS 17 for now (#2627) via NachoSoto (@NachoSoto)
* `Tests`: fixed crash on iOS 13 (#2624) via NachoSoto (@NachoSoto)
* `StoreTransaction`: read `Storefront` from `StoreKit.Transaction`
(#2611) via NachoSoto (@NachoSoto)
* `StoreKitConfigTestCase`/`BaseStoreKitIntegrationTests`: also clear
transactions after every test (#2616) via NachoSoto (@NachoSoto)
* `ErrorCode.networkError`: improved description (#2610) via NachoSoto
(@NachoSoto)
* `PurchaseTester`: make CI job always point to current version (#2622)
via NachoSoto (@NachoSoto)
* Improved `finishAllUnfinishedTransactions` (#2615) via NachoSoto
(@NachoSoto)
* `StoreKitConfigTestCase`: improved `waitForStoreKitTestIfNeeded`
(#2614) via NachoSoto (@NachoSoto)
* `StoreKitConfigTestCase`: set `continueAfterFailure` to `false`
(#2617) via NachoSoto (@NachoSoto)
* `PaywallExtensions`: fixed compilation (#2613) via NachoSoto
(@NachoSoto)
* `CI`: added `iOS 17` job (#2591) via NachoSoto (@NachoSoto)
* `Encodable.jsonEncodedData`: fixed tests on iOS 17 due to inconsistent
key ordering (#2607) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: added ability to display new
`SubscriptionStoreView` (#2595) via NachoSoto (@NachoSoto)
* Refactor: extracted all log strings (#2600) via NachoSoto (@NachoSoto)
* Changed tests to work around `URL` decoding differences in `iOS 17`
(#2605) via NachoSoto (@NachoSoto)
* Removed unnecessary `Strings.trimmedOrError` (#2601) via NachoSoto
(@NachoSoto)
* Fixed test compilation with `Xcode 15` (#2602) via NachoSoto
(@NachoSoto)
* Tests: added `iOS 17` snapshots (#2603) via NachoSoto (@NachoSoto)
* `StoreProductDiscount`: added `description` (#2604) via NachoSoto
(@NachoSoto)
* `debugRevenueCatOverlay` improvements (#2594) via NachoSoto
(@NachoSoto)
* `Xcode 15`: fixed all documentation warnings (#2596) via NachoSoto
(@NachoSoto)
* `StoreKitObserverModeIntegrationTests`: fixed and disabled SK2
`testPurchaseInDevicePostsReceipt` (#2589) via NachoSoto (@NachoSoto)
* `StoreKit2TransactionListener`: added log when receiving
`Transactions.Updates` (#2588) via NachoSoto (@NachoSoto)
* `Dictionary.MergeStrategy`: simplify implementation (#2587) via
NachoSoto (@NachoSoto)
* `Configuration.Builder`: fixed doc reference (#2583) via NachoSoto
(@NachoSoto)
* `APITesters`: available since iOS 11 (#2581) via NachoSoto
(@NachoSoto)
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.

2 participants