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

docs: update docker development instructions #307

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

eatyourgreens
Copy link
Contributor

When you're developing locally with Docker, you need to install the course materials first.

@alasdairwilson
Copy link
Collaborator

So there is supposed to be flexibility here in terms of a build-arg, if you pass material_method=pull it then pulls the material, default is copy which means copying it from the base directory on build, but I think this should be explained in the docs (and probably tested) in a separate PR.

Copy link
Collaborator

@alasdairwilson alasdairwilson left a comment

Choose a reason for hiding this comment

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

Actually, the reason why we addd the python script to pull material is so you didnt need the node/js stuff installed to do it.

Since the point of this is developing in the container, pontentially because you don't want to install it locally, could you change this to use the python script which almost certainly is installed, or at least to point out this alternative exists.

@eatyourgreens
Copy link
Contributor Author

.material is mounted as a volume in local dev, so that you can edit the course material locally (rather than having to edit files inside the running container.)

@alasdairwilson
Copy link
Collaborator

Yeah sorry, I get that, it just used to do that automatically (populate the material folder before build) as long as you told it to pull with the build arg. Unless that functionality has been broken with subsequent changes.

In any sense, the suggestion was to swap this to the python script since the whole point of the docker environment is to not have to install the node packages locally since yarn pullmat requires node, ts, etc. to be there via a yarn install to be run.

Since we can't separate out the specific dependencies in that script from the project (the git dependency) sensibly and, even if we did you would still need to install node first, whereas python should be on most anyone's system and we define the only 2 smalkl depencies (gitpython and pyyaml) in the requirements.txt file, it makes much more sense to use that to pre-pull material rather than the ts script.

@eatyourgreens
Copy link
Contributor Author

Ah right, got you. Still got some problems here trying to help the ICR people get Gutenberg up and running from a clean install for the hackday. 😞

When you're developing locally with Docker, you need to install the course materials first.
@eatyourgreens eatyourgreens force-pushed the update-docker-dev-docs branch from 1203668 to 2a9a973 Compare February 7, 2025 15:20
@eatyourgreens
Copy link
Contributor Author

I've changed it to python scripts/pull_material.py.

@alasdairwilson alasdairwilson merged commit d81e348 into main Feb 10, 2025
4 checks passed
@eatyourgreens eatyourgreens deleted the update-docker-dev-docs branch February 10, 2025 15: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