-
Notifications
You must be signed in to change notification settings - Fork 44
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
👻 Default to production for NODE_ENV #1502
Conversation
82f2252
to
3ce1e39
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1502 +/- ##
==========================================
+ Coverage 40.16% 40.60% +0.43%
==========================================
Files 144 145 +1
Lines 4551 4586 +35
Branches 1096 1106 +10
==========================================
+ Hits 1828 1862 +34
- Misses 2626 2627 +1
Partials 97 97
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: ibolton336 <[email protected]>
2043eaf
to
2ce4d7d
Compare
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.
Yeah, the destructuring in buildKonveyorEnv
to convert process.env to the internal ENV was filtering out optional values that I hadn't included.
Didn't appear to impact the issue at hand, but is nice to have the defaults explicitly included to enhance the dev experience. Seems that the result of buildKonveyorEnv includes all of the env vars available from process.env at runtime even if the type definition constrains the expected properties. TypeScript type checking occurs at compile-time, and it doesn't prevent you from accessing properties not explicitly defined in the type. If we wanted to ensure that only the expected properties are included and to have stricter type checking, we could explicitly specify the properties we want from process.env
|
Being explicit about what the `NODE_ENV` value should be for each npm script where the value could impact the results can help remove any errors in future. Motivated by working on konveyor#1502. Signed-off-by: Scott J Dickerson <[email protected]>
Being explicit about what the `NODE_ENV` value should be for each npm script where the value could impact the results can help remove any errors in future. Motivated by working on #1502. Signed-off-by: Scott J Dickerson <[email protected]>
Fixes https://issues.redhat.com/browse/MTA-1599