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

Fix line endings & add docker build warnings #12

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

jamievlin
Copy link
Member

No description provided.

@jamievlin jamievlin requested a review from No767 November 12, 2023 21:12
@jamievlin jamievlin self-assigned this Nov 12, 2023
Comment on lines 48 to 49
RUN test -f /rodhaj/bot/.env \
|| (echo "Environment file not found; please copy .env to bot/.env"; exit 1)
Copy link
Member

Choose a reason for hiding this comment

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

The idea of having the .env file is to only use it during the development lifecycle, and during prod, all we have to do is to pass in a pre-determined .env and populate the container with the env.

Although I did say Windows development is not really supported, it should be fine to develop on as i checked and all wheels are built also for windows. Winloop is used in place of uvloop in order to aid with Windows support. Note that I haven't tested it on Windows, hence the warning

Copy link
Member Author

Choose a reason for hiding this comment

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

Wait. Do you want me to remove this line or modify it somehow?

Copy link
Member

@No767 No767 Nov 12, 2023

Choose a reason for hiding this comment

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

Basically in a nutshell, I'm just saying you should remove it. (The bot will actually read off of the environment variables set in the shell if it can't find the .env file)

Copy link
Member Author

@jamievlin jamievlin Nov 12, 2023

Choose a reason for hiding this comment

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

Ah okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the commit

.gitattributes Show resolved Hide resolved
Copy link
Member

@No767 No767 left a comment

Choose a reason for hiding this comment

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

LGTM

@jamievlin jamievlin merged commit 71bff5f into main Nov 12, 2023
5 checks passed
@jamievlin jamievlin deleted the dev/jamievlin/fix-sh-lineends branch November 12, 2023 21:37
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.

2 participants