-
Notifications
You must be signed in to change notification settings - Fork 1
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
Disable analytics in OSS code #558
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
=======================================
Coverage 84.62% 84.62%
=======================================
Files 25 25
Lines 1171 1171
=======================================
Hits 991 991
Misses 180 180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Dockerfile
Outdated
@@ -46,4 +46,7 @@ RUN apt-get remove -y \ | |||
cpp-12 \ | |||
cpp | |||
|
|||
# Disable analytics in OSS | |||
RUN export GX_ANALYTICS_ENABLED=false |
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.
Any reason to not use the ENV
instruction? https://docs.docker.com/reference/dockerfile/#env
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.
Oh good catch, you should use ENV. Look at this example: https://stackoverflow.com/questions/33379393/docker-env-vs-run-export
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.
Didn't know that existed. Updating
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.
Just a question, but if this works no need to block.
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.
LGTM, thanks!
See for context: https://github.com/greatexpectationslabs/infrastructure/pull/311/files#r1854502407