-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix env vars #1645
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
Fix env vars #1645
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes look good, QA Passed 👍
app/core/Analytics.js
Outdated
if (!__DEV__) { | ||
RCTAnalytics.trackEvent({ | ||
...event, | ||
...params, | ||
value, | ||
info | ||
}); | ||
} else { | ||
Logger.log(`Analytics 'trackEventWithValueAndParameters' - `, event, value, info, params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this could be a method? looks like we're dong the same thing 6x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only recommendation I have is to put code like this:
if (!__DEV__) {
RCTAnalytics.trackEvent(event);
} else {
Logger.log(`Analytics 'trackEvent' - `, event);
}
On a function outside on a service or helper that would look like:
const trackEvent = event => {
if (!__DEV__) {
RCTAnalytics.trackEvent(event);
} else {
Logger.log(`Analytics 'trackEvent' - `, event);
}
}
And then you could call trackEvent(event)
everywhere so we don't repeat code. This is especially important if we need to use that function more times in the future.
And the same for !__DEV__ && RCTAnalytics.optIn(this.enabled);
Otherwise looks good to me!!
@rickycodes @andrepimenta it totally makes sense, it was that way before so I didn't question it 🤦 |
* enable-apple-pay * analytics only on not dev * startwatcher * dev * env. * DRY analytics
Description
Env vars haven't been working on debug mode for a while now, this PR fixes that so we can use all the features on debug mode.
Checklist
Issue
Resolves #???