You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I just noticed that I failed to give feedback on #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.
The text was updated successfully, but these errors were encountered:
I just noticed that I failed to give feedback on #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 calledpurchaseCompleted
. Similarly, I would suggest we rename this new API right away (before too many people use it) to benavigated
,navigationPathChanged
,navigationChanged
, or at leastnavigation
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 prefernavigationPathChanged
because that would be consistent with the signal nameNavigation.pathChanged
which is the exact same logic I applied for the other helpers which were namedError.occurred
andPurchase.completed
respectively.I've also noticed you placed the helpers right inside the
TelemetryDeck.swift
file whereas the other helpers are in respective extensions namedTelemetryDeck+Errors.swift
andTelemetryDeck+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.swift
file should really focus on the main functionality, which is mainly thesignal
function.@winsmith Please let me know what you think about the rename.
The text was updated successfully, but these errors were encountered: