Skip to content
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

Feature: Give option to specify the directory where envfile will be created #5

Merged
merged 3 commits into from
Nov 22, 2020

Conversation

sumit4613
Copy link
Contributor

This PR closes #4
Adds support to specify the directory where envfile will be created.

@AngelOnFira Please update this PR if any change is required.

Thanks!

Copy link

@iamhssingh iamhssingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment about using os.path.join

text_file.write(out_file)
else:
with open(f"/github/workspace/{directory}/{file_name}", "w") as text_file:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use os.path.join


if "None" in directory:
with open(f"/github/workspace/{file_name}", "w") as text_file:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use os.path.join

@iamhssingh
Copy link

Also, can you squash changes? @sumit4613

fix env variable name

use f-strings

fix typing mistake

update readme

handles a case when user want to create env file at root

update readme

update readme and prepare to raise PR
Copy link

@ZarcoNontol ZarcoNontol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@AngelOnFira AngelOnFira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took me so long to get around to the review. I noted a few things about the implementation.

README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -21,21 +21,24 @@ jobs:

steps:
- name: Make envfile
uses: SpicyPizza/create-envfile@v1
uses: SpicyPizza/create-envfile@master
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/create-envfile.py Outdated Show resolved Hide resolved
@sumit4613
Copy link
Contributor Author

sumit4613 commented Nov 22, 2020

@AngelOnFira I had updated this PR with all required changes. Please let me know if I had left anything.
Thanks!

@AngelOnFira AngelOnFira self-requested a review November 22, 2020 05:39
Copy link
Member

@AngelOnFira AngelOnFira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 💯

@AngelOnFira
Copy link
Member

Thanks for the work @sumit4613 😃

@AngelOnFira AngelOnFira merged commit 1a74572 into SpicyPizza:master Nov 22, 2020
@AngelOnFira
Copy link
Member

I'll try to create an updated release in the next few days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add directory support for env file
4 participants