Skip to content
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

Env variables #813

Merged
merged 5 commits into from
May 28, 2021
Merged

Env variables #813

merged 5 commits into from
May 28, 2021

Conversation

SamLee514
Copy link
Contributor

Summary

Moving sentry url from strings.json to environment variable after I had been unknowingly spamming vaken status for the past month and a half.

Changes GCP bucket template string into one-liner as per README

Remove contributoors, replace with VandyHacks

Review Focus

Anything you want the reviewer to pay attention to? Feel free to delete this section.

@SamLee514 SamLee514 requested review from aadibajpai and leonm1 March 29, 2021 19:13
@codeclimate
Copy link

codeclimate bot commented Mar 29, 2021

Code Climate has analyzed commit bb63e68 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (0% is the threshold).

This pull request will bring the total coverage in the repository to 63.4% (0.0% change).

View more on Code Climate.

@SamLee514 SamLee514 requested a review from bencooper222 March 29, 2021 23:31
.env.template Outdated
"auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", \
"client_x509_cert_url": "acerturl" \
}'
GCP_STORAGE_SERVICE_ACCOUNT='{"type":"service_account","project_id":"foo","private_key_id":"aprivatekey","private_key": "anotherprivatekey","client_email": "[email protected]","client_id": "aclientid","auth_uri": "https://accounts.google.com/o/oauth2/auth","token_uri": "https://oauth2.googleapis.com/token","auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs","client_x509_cert_url": "acerturl"}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to break this up? Makes it easier to read?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps leave a comment above it reminding whoever uses the template to remove the spaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, rationale was that I was a dumdum and just assumed the formatting was how it was supposed to be and disregarded the README but that's fair point, will revert

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh this is probably revealing of the issue of relying on .env files. If you could get #755 done, a lot of your problems would be fixed.

@github-actions github-actions bot added the approved Automatically added, do not manually add label Mar 30, 2021
src/client/index.tsx Outdated Show resolved Hide resolved
@SamLee514
Copy link
Contributor Author

Straight up forgot about this for a hot minute lol @leonm1 these changes look good?

Copy link
Member

@leonm1 leonm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SamLee514 SamLee514 merged commit 72523be into main May 28, 2021
@SamLee514 SamLee514 deleted the env-variables branch May 28, 2021 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Automatically added, do not manually add
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants