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

Introducing Docker Compose for Enhanced Development Experience #251

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

teamchong
Copy link

This pull request introduces Docker Compose to the existing Docker setup:

  • Added a docker-compose.yml file to simplify container orchestration and environment setup.
  • Ensured proper volume binding and environment variable management with Docker Compose.
  • Modified wiki.md to guide users on utilizing Docker Compose and the flexibility it offers, such as changing the default online_log/app.py.

This PR aim to further enhance the developer experience, making it easier for new users to get started.

2023-11-02 at 22 23 28 - Blue Starfish

Feedback and testing are welcome.

Copy link
Contributor

@ManindraDeMel ManindraDeMel left a comment

Choose a reason for hiding this comment

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

I think these are some good changes. However, as mentioned in my specific comments I think it shouldn't run the web app by default. I looked into adding this functionality but there were a couple errors. I'll look into it more when I have some free time. Otherwise it's a good addition to the project! Let me know of your thoughts on my aforementioned points!

Dockerfile Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
wiki.md Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@ManindraDeMel ManindraDeMel left a comment

Choose a reason for hiding this comment

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

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

wiki.md Outdated Show resolved Hide resolved
wiki.md Show resolved Hide resolved
dev.Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@ManindraDeMel ManindraDeMel 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

@Bioblaze
Copy link

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

this is a bad idea, most .gitignores should have .env by default. having .env.sample or even sample.env would be good to have, legit for the long run. When your running in production or development environments for teams and such.

@ManindraDeMel
Copy link
Contributor

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

this is a bad idea, most .gitignores should have .env by default. having .env.sample or even sample.env would be good to have, legit for the long run. When your running in production or development environments for teams and such.

This is a good point, but I've never seen an env.sample or sample.env used personally. the environment can just be defined in the documentation

@Bioblaze
Copy link

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

this is a bad idea, most .gitignores should have .env by default. having .env.sample or even sample.env would be good to have, legit for the long run. When your running in production or development environments for teams and such.

This is a good point, but I've never seen an env.sample or sample.env used personally. the environment can just be defined in the documentation

Well this isn't for the actual environment its more for storage in the repo, and then we add the different .env production.env, etc into the gitignore.

This way we can just use cp example.env .env and we can put our envs in there, and not worry about it ever accidently getting pushed. Cause you know someone is going todo it. Hell we all have done it at 1 point or another.

@ManindraDeMel
Copy link
Contributor

ManindraDeMel commented Dec 23, 2023

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

this is a bad idea, most .gitignores should have .env by default. having .env.sample or even sample.env would be good to have, legit for the long run. When your running in production or development environments for teams and such.

This is a good point, but I've never seen an env.sample or sample.env used personally. the environment can just be defined in the documentation

Well this isn't for the actual environment its more for storage in the repo, and then we add the different .env production.env, etc into the gitignore.

This way we can just use cp example.env .env and we can put our envs in there, and not worry about it ever accidently getting pushed. Cause you know someone is going todo it. Hell we all have done it at 1 point or another.

Yeah, alright sure! I don't really mind it's a minor thing.

@cryptictech
Copy link

cryptictech commented Jan 2, 2024

Good changes, just a slight redundancy with the .env.sample. Just leave it as .env

this is a bad idea, most .gitignores should have .env by default. having .env.sample or even sample.env would be good to have, legit for the long run. When your running in production or development environments for teams and such.

This is a good point, but I've never seen an env.sample or sample.env used personally. the environment can just be defined in the documentation

Laravel (docs) is a good example of .env.example (in their case) where the official instruction is to cp .env.example .env

Copy link
Collaborator

@XiaoDu-flying XiaoDu-flying left a comment

Choose a reason for hiding this comment

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

It is stimulating to see all your chats, however there are some conflicts between your work and chatdev in Experiential Co-Learning function, we are looking forward that these conflicts are solved.

@ManindraDeMel
Copy link
Contributor

Please fix the conflicts

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.

5 participants