Uh oh!
There was an error while loading. Please reload this page.
[docs] Warn/clarify that env vars are NOT "SECRET" - #6062
Conversation
JBallin
commented
Dec 28, 2018
Lines 26-30 (in my branch, pasted below) need to be adjusted as well, but I would like some clarity. I'm uncertain why there are such deliberate assertions that secrets can be used in env vars, am I misunderstanding something? I would also like some insight into when not to include env vars in version control.
|
petetnt
commented
Dec 30, 2018
I think the |
petetnt
left a comment
There was a problem hiding this comment.
Few tiny comments, all in all I think this is a pretty good change and something that seems to trip many new CRA users (including people in my team).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Thanks for the review @petetnt, incorporated your suggestions. Still unsure about this line:
|
petetnt
commented
Jan 6, 2019
I think that the whole sentence could be removed, it doesn't offer much / anything that wasn't explained in the document beforehand. |
Uh oh!
There was an error while loading. Please reload this page.
JBallin
commented
Jan 7, 2019
@petetnt I actually like the first part of the sentence (up until "deployed"), but I'm still curious as to why version control was brought up.
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. |
I’m still waiting for a response from the maintainers, so this should be kept open. |
ianschmitz
left a comment
There was a problem hiding this comment.
Thanks for this @JBallin. Just one small tweak and I think we should be good.
Uh oh!
There was an error while loading. Please reload this page.
Fixes#5676 Co-Authored-By: Ian Schmitz <ianschmitz@gmail.com>
ianschmitz
commented
Feb 10, 2019
Thanks @JBallin! |
JBallin
commented
Feb 10, 2019
Thank YOU @ianschmitz, I'm so excited to have made my first contribution to CRA! Unfortunately, an unintended change snuck through. I've opened a PR (#6386) to fix it. Sorry about that. |
Fixes#5676