chore: remove custom testflight module #7082
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
🧪 Flashlight Performance Report (AWS Device Farm) 🔀 Commit: db9ad8c
|
| enableAutoSessionTracking: false, | ||
| enableTracing: false, | ||
| environment: isTestFlight ? 'Testflight' : SENTRY_ENVIRONMENT, | ||
| environment: IS_TEST_FLIGHT ? 'Testflight' : SENTRY_ENVIRONMENT, |
There was a problem hiding this comment.
nit: TestFlight, as I see we're using SENTRY_ENVIRONMENT=LocalRelease for example with CamelCase (also consistent with getInstallerPackageNameSync())
There was a problem hiding this comment.
Would agree but I think this would cause us to have two environments show up in Sentry, "TestFlight" and "Testflight". We would have to just ignore the old one. Not the worst thing, but not sure it's worth it.
| candlestick_charts_enabled: IS_DEV || isTestFlight || false, | ||
| rainbow_toasts_enabled: IS_DEV || isTestFlight || false, | ||
| candlestick_charts_enabled: IS_DEV || IS_TEST_FLIGHT || false, | ||
| rainbow_toasts_enabled: IS_DEV || IS_TEST_FLIGHT || false, |
There was a problem hiding this comment.
should we remove || false here as all previous types are strictly boolean?
i1skn
left a comment
i1skn
left a comment
There was a problem hiding this comment.
Overall LGTM, see just small nits
olerass
left a comment
olerass
left a comment
There was a problem hiding this comment.
It's a bit anti-pattern we even have such a var to begin with, and switching code behavior based on it. BUT with that said, this change is still a good improvement 🙏 /boy-scout-rule
What changed (plus any additional context for devs)
react-native-device-infoto latest versionreact-native-device-infoenv.tswith other similar constants.