Fix/url analytics property name #670
Conversation
| techVersion: techVersionDefault, | ||
| product: productDefault | ||
| } = getSDKVersions(); | ||
| const sdkCode = ensureOption(options, 'sdkCode', ensureOption(options, 'sdk_code', sdkCodeDefault)); |
There was a problem hiding this comment.
@cloudinary-pkoniu Maybe it's worth introducing a new helper function, like ensureOptionWithFallback or something like that. Or make this function more generic, instead of single name to get a list of names.
There was a problem hiding this comment.
it'd be more like "ensureOptionWithFallbackWithFallback", I can try to see if function currying accomplishes the same thing and is readable but for now let's merge and have that released
There was a problem hiding this comment.
it's approved from my side, let's see what @magdakwiecien says
There was a problem hiding this comment.
I see no problem in "nesting" it like this, I understood how it works quickly, IMO it's even better to use composition here instead of introduce additional helpers.
Brief Summary of Changes
Previously added
urlAnalyticsis not consistent with the naming convention we had so far in the repo. I addedanalyticsside by side. If someone usedurlAnalyticsearlier and is also usinganalytics, the older takes precedence.What Does This PR Address?
Are Tests Included?
Reviewer, Please Note: