Add support for media dir environment variable. #26
Conversation
README.md
Outdated
| $ git-sim --media-dir=path/to/output status | ||
| ``` | ||
|
|
||
| Optionally, set the environment variable `git_sim_media_dir` to default that as the media directory, if no `--media-dir` is provided. |
There was a problem hiding this comment.
Let's update to something like:
Optionally, set the environment variable git_sim_media_dir to set a global default media directory, to be used if no --media-dir is provided. Simulated output images/videos will be placed in this location, in subfolders named with the corresponding repo's name.
README.md
Outdated
| $ export git_sim_media_dir=path/to/media/directory | ||
| $ git-sim status | ||
| ``` | ||
| Note: `--media-dir` takes precedence over the environment variable. If you set the environment and still provide the argument, you'll find the media in the path provided by `--media-dir` |
There was a problem hiding this comment.
Add a period at the end of the second sentence.
git_sim/__main__.py
Outdated
| config.verbosity = "ERROR" | ||
|
|
||
| # If the env variable is set and no argument provideed, use the env variable value | ||
| if os.getenv('git_sim_media_dir') is not None and args.media_dir == '.': |
There was a problem hiding this comment.
I think we can shorten the if statement if we remove the is not None and just use if os.getenv('git_sim_media_dir') and args.media_dir == '.':
git_sim/__main__.py
Outdated
| config.media_dir = os.path.join(os.path.expanduser(args.media_dir), "git-sim_media") | ||
| config.verbosity = "ERROR" | ||
|
|
||
| # If the env variable is set and no argument provideed, use the env variable value |
There was a problem hiding this comment.
Typo - "provideed" instead of "provided"
initialcommit-io
left a comment
initialcommit-io
left a comment
There was a problem hiding this comment.
Looks like there are multiple similar commits here in this pull request, can you squash it into just 1? And make the changes I requested in the my comments?
Updated the README to reflect the env variable changes. Signed-off-by: Abhijit Nathwani <abhijit.nathwani@gmail.com>
Signed-off-by: Abhijit Nathwani <abhijit.nathwani@gmail.com>
abhijitnathwani
force-pushed
the
add-env-variable-for-media-dir
branch
from
eee3ef3 to
c932a4e
Compare
|
Thanks for the review! I have updated it as per your comments. |
2 participants
@initialcommit-io as discussed, I have implemented the changes for setting the environment variable. Let me know if it requires any other updates. I have tested on Windows/Linux. I don't have access to a Macintosh to test it. Can you please verify this once ?
Fixes #4