-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
Map environment #62
Map environment #62
Conversation
/rebuild-readme |
/test all |
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 nitpick
Co-authored-by: Andriy Knysh <[email protected]>
/rebuild-readme |
/test all |
@aknysh I used the same variable description as the upstream module. We might want to change it upstream too. I merged your changes, rebuilt the readme, and reran the tests. Looks good! Mind taking another look? |
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.
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 just the map_environment variable name ad @osterman pointed out
ah yes, too bad on the inconsistent naming. Do we have a consistent naming document we can refer to ? Might also help to add this document as a checkbox on new PRs within the PR template |
what
map_environment
argument of the container definition module to make the environment variables more readablewhy
references