-
Notifications
You must be signed in to change notification settings - Fork 16
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
[ Issue 389 ] Add Google Analytics #453
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.
Looks great, thanks. Makes the most sense to include all envs now, even if we remove them later.
const NEXT_ENVS = ["development", "test", "production"] as const; | ||
export type NextPublicAppEnv = (typeof NEXT_ENVS)[number]; | ||
|
||
const CURRENT_ENV = process.env.NODE_ENV ?? "development"; |
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.
Where is NODE_ENV
being set? Our application is environment agnostic at the moment, and prefer it that way (have intentionally made decisions to maintain that). Just not sure this will hit anything but "development"
at this point.
EDIT: Thanks for explaining offline that it's just how we start the app and what's happening with AWS accounts both being production.
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.
NODE_ENV
is set when we run npm run start
automagically by node to development
and that is a flag that node uses to set certain configs. npm run test
sets it to test
, and serving it from the server.js
file sets it to production
. We could have more finely tuned control by using env files, but that would add some overhead that I don't think we need yet.
Not sure I understand why AWS dev environment is reported to production. So "dev" is local development, what is test? |
Summary
Fixes #389
Time to review: 5 minutes
Changes proposed
Context for reviewers
Because we're using the node environment to distinguish between dev, test, and prod, our AWS dev environment will be reported with production. The dev metrics will come from local development environments.
Waiting on admin access to our analytics account to configure dev and test enviornments.Waiting on admin access to our analytics account to configure DAP directly in the GA console