-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Feature: Added devcontainer support #1252
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
base: master
Are you sure you want to change the base?
Feature: Added devcontainer support #1252
Conversation
Hey @Naramsim, |
Hi, I suggest two things:
|
in my case the HTTPS port didn't work and the 80 was mounted on http://localhost:35499/api/v2/ . So a bit counterintuitive. Secondly, the git inside the container not usable, nor detected by VSCode that prompts me to install git. I think this devcontainers aren't really useful afterall, honestly. |
Hey, I will work on fixing these issues. After that, we can decide whether to keep or remove the devcontainer based on what works best. |
@Naramsim I have fixed the issues and added instructions in the CONTIRUBTING.md. Do check it out and tell me if you face any issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, Can't we use this override https://github.com/PokeAPI/pokeapi/blob/master/docker-compose.override.yml ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image from Docker Hub automatically starts the Gunicorn server, but for our Dev Container setup, we actually don't want it running by default. I've overridden the CMD to prevent that, as we use make
commands to handle server startup.
To keep the main docker-compose.override.yml
clean for its original purpose, I set up a separate override file just for the Dev Container here: .devcontainer/docker-compose-override.yml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Dev Container features like docker-outside-of-docker (and others), we typically need a Debian-based image for them to work properly.
Since our current image is built on Alpine Linux, I would suggest creating a separate Dockerfile
inside the .devcontainers folder. This dedicated Dockerfile
would then build our Dev Container image using a Debian base.
What do you think of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, honestly I think it's too much work with few benefits :) If you like you could work on other issues.
Top two would be: helping @Indyandie with the new Astro website, polishing the openapi schema and make it available in the current website
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Naramsim,
Totally get where you're coming from on keeping things simple, and I completely agree that we shouldn't add complexity without good reason.
But, just wanted to quickly mention why I think Dev Containers could actually save us headaches long-term: it means anyone can jump in and contribute instantly, without wrestling with setup. It ensures everyone's development environment is the same, so we avoid "it works on my machine" issues. Plus, it works well with features like cloud development (like GitHub Codespaces).
It feels like a small upfront effort for a much smoother ride for all contributors, especially new ones.
If you still think this isn't required, I'll totally drop it and focus on other things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @PraaneshSelvaraj sorry for coming back only now. I'm currently unable to fully review the devcontainer file honestly. Maybe I'll just merge it in and see if issues arise when people start using it. Still unsure. I have limited time currently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Naramsim , Sounds good.
What’s Added
This PR adds support for Dev Container. It makes it easy to get started with PokeAPI in a consistent, containerized development environment.
Highlights
.devcontainer/devcontainer.json
:docker-compose.yml
anddocker-compose-dev.yml
How to Use
Ctrl+Shift+P
orCmd+Shift+P
) and run:Dev Containers: Reopen in Container
Why It’s Useful
Fixes #1251