I just noticed that I failed to give feedback on https://github.com/TelemetryDeck/SwiftSDK/pull/161 about the confusing naming of the newly suggested functions (to my defense, I was still in jet lag and not fully back to work yet). A call named TelemetryDeck.navigate(from: "Source.path", to: "Destination.path") reads like an API which executes navigation because it's using the imperative form. Users might think it helps with SwiftUI navigation or similar, which isn't the case.
The preset helpers all use a past form to indicate that "something happened", not "something will happen thanks to this call". The error function is named errorOccurred and the purchases one is called purchaseCompleted. Similarly, I would suggest we rename this new API right away (before too many people use it) to be navigated, navigationPathChanged, navigationChanged, or at least navigation if you don't like the others (which isn't technically past, but at least it doesn't sound like it executes navigation). I personally prefer navigationPathChanged because that would be consistent with the signal name Navigation.pathChanged which is the exact same logic I applied for the other helpers which were named Error.occurred and Purchase.completed respectively.
I've also noticed you placed the helpers right inside the TelemetryDeck.swift file whereas the other helpers are in respective extensions named TelemetryDeck+Errors.swift and TelemetryDeck+Purchases.swift. I could adjust the name with a fix-it provided (at least for a short time) and I also could move the code to an extra file for consistency. I think the main TelemetryDeck.swift file should really focus on the main functionality, which is mainly the signal function.
@winsmith Please let me know what you think about the rename.
I just noticed that I failed to give feedback on https://github.com/TelemetryDeck/SwiftSDK/pull/161 about the confusing naming of the newly suggested functions (to my defense, I was still in jet lag and not fully back to work yet). A call named
TelemetryDeck.navigate(from: "Source.path", to: "Destination.path")reads like an API which executes navigation because it's using the imperative form. Users might think it helps with SwiftUI navigation or similar, which isn't the case.The preset helpers all use a past form to indicate that "something happened", not "something will happen thanks to this call". The error function is named
errorOccurredand the purchases one is calledpurchaseCompleted. Similarly, I would suggest we rename this new API right away (before too many people use it) to benavigated,navigationPathChanged,navigationChanged, or at leastnavigationif you don't like the others (which isn't technically past, but at least it doesn't sound like it executes navigation). I personally prefernavigationPathChangedbecause that would be consistent with the signal nameNavigation.pathChangedwhich is the exact same logic I applied for the other helpers which were namedError.occurredandPurchase.completedrespectively.I've also noticed you placed the helpers right inside the
TelemetryDeck.swiftfile whereas the other helpers are in respective extensions namedTelemetryDeck+Errors.swiftandTelemetryDeck+Purchases.swift. I could adjust the name with a fix-it provided (at least for a short time) and I also could move the code to an extra file for consistency. I think the mainTelemetryDeck.swiftfile should really focus on the main functionality, which is mainly thesignalfunction.@winsmith Please let me know what you think about the rename.