-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Switch from .reporterrors file to flag + envvar #1697
Conversation
65e314d
to
802f413
Compare
247fe54
to
7d58deb
Compare
802f413
to
f09857a
Compare
7d58deb
to
e984aa3
Compare
I wanted some way to persist the user's choice since in the non-interactive case, I imagined a user coming along, running the updated installer, making a choice, then completely forgetting the choice was made in the future. The persisted file would avoid users having to remember every time that they need to opt in or out of error collection. I expect if we don't prompt them every month users are more likely to stay opted-in to error collection. Perhaps a more robust solution is a config file which is similar to a .env file (each option is just the environment variable setting?). e.g. they can put |
I think we should also update the readme for this to include information here |
What's the harm in using the .env file? |
The .env files is more meant for runtime, not install time. Also if a user does the following things could break:
Also, ideally we'd be able to persist the error collection choice of users, but updating the .env file would be harder to do. |
f09857a
to
8141e1d
Compare
Ah yeah, I guess it's better to leave installation configs separate from runtime |
5b9b2ae
to
e59fce0
Compare
e984aa3
to
e3b8712
Compare
d0532d0
to
fbc1f06
Compare
Automerge enabled! Here we gooooo! |
I did some bikeshedding on the naming of the flags/vars by looking at parallels with |
The
.reporterrors
file is a somewhat idiosyncratic way to configure the sentry installer. We use environment variables and CLI flags for other configuration, we don't have a proper configuration file for the installer itself (vs. the runtime config files insentry/
). This PR switches to using envvars and CLI flags to configure whether we report to Sentry.This also expands the scope of the ask to allow for future expansion into runtime data collection. We're starting with just the installer but we don't want to have to go through the consent exercise again in the future if we don't have to, better to ask once up front here.