Update readme.md - different env in ios #281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The use of /tmp/envfile is a bad idea, especially when there is one machine doing multiple builds on the same machine in parallel (eg. build server). IMHO it is much better to copy the correct .env.staging file to the default .env file. In this way we are not modifying system-wide files like /tmp/envfile. Do you guys agree?
luancurti
approved these changes
Jun 13, 2020
| @@ -139,11 +139,9 @@ Then edit the newly created scheme to make it use a different env file. From the | |||
| - Click "Pre-actions", and under the plus sign select "New Run Script Action" | |||
| - Where it says "Type a script or drag a script file", type: | |||
| ``` | |||
| echo ".env.staging" > /tmp/envfile # replace .env.staging for your file | |||
| cp ${PROJECT_DIR}/../.env.staging .env # replace .env.staging for your file | |||
There was a problem hiding this comment.
- Shouldn't the destination be ${PROJECT_DIR}/../.env then?
- there is another reference to "/tmp/envfile" up the text
- in terms of implementation /tmp/envfile takes priority, however this is nowhere documented; I would expect either to document it or to remove it from the code entirely
There was a problem hiding this comment.
The /tmp/envfile priority is definitely confusing especially if switching to the new recommendation on a CI server where past builds populated /tmp/envfile.
|
@jslowack Following the updated readme, I get |
jjd314
added a commit
to jjd314/react-native-config
that referenced
this pull request
Jul 24, 2020
There's a comment as part of the review of pull request lugg#281 by @danielwinkler that the build pre-action for iOS should copy the .env.staging (or whatever) file to ${PROJECT_DIR}/../.env, which turns out to be correct. This change updates the README to reflect that (and also addresses his other comment about the second reference to /tmp/envfile). @maharjanaman also mentioned on that pull request a problem with the script, and I believe that was from not having a "Provide build settings from" selection, so I added a comment about that, as well.
luancurti
pushed a commit
that referenced
this pull request
Aug 4, 2020
There's a comment as part of the review of pull request #281 by @danielwinkler that the build pre-action for iOS should copy the .env.staging (or whatever) file to ${PROJECT_DIR}/../.env, which turns out to be correct. This change updates the README to reflect that (and also addresses his other comment about the second reference to /tmp/envfile). @maharjanaman also mentioned on that pull request a problem with the script, and I believe that was from not having a "Provide build settings from" selection, so I added a comment about that, as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of /tmp/envfile is a bad idea, especially when there is one machine doing multiple builds on the same machine in parallel (eg. build server).
IMHO it is much better to copy the correct .env.staging file to the default .env file. In this way we are not modifying system-wide files like /tmp/envfile.
Do you guys agree?