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

Possible outdated Docker command to update JS and CSS #4540

Closed
drakene opened this issue Feb 4, 2021 · 2 comments · Fixed by #4541
Closed

Possible outdated Docker command to update JS and CSS #4540

drakene opened this issue Feb 4, 2021 · 2 comments · Fixed by #4541
Labels
Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Type: Bug Something isn't working. [managed]

Comments

@drakene
Copy link
Contributor

drakene commented Feb 4, 2021

The command below doesn't work on my Windows build for remaking CSS/JS assets. The "Home" container, in fact, shuts down after a short while of executing "docker-compose up."

possible_old_docker_command

However, on Gitter Yash Saravgi @Yashs911 said that the home container shutting down is normal behavior and that the command to remake CSS assets was the following:

docker-compose exec web make css  
docker-compose exec web make js  

I wanted to open an issue for this just in case there is either an problem on the wiki (which I would be willing to fix and make a pull request for if necessary) or if there is something going wrong on the Windows build. I'm still new, so it could be a mistake on my end. With that being said, Yash's solution works for me. Please let me know if I can make this issue more descriptive or neater for future reference!

Relevant url?

https://github.com/internetarchive/openlibrary/blob/master/docker/README.md

Steps to Reproduce

  1. Get the local build up and running
  2. Make some changes to CSS/JS
  3. In terminal using working directory of local repo, type: docker-compose exec home npm run build-assets

Outcome

  • Actual: Output in terminal of "No container found for home_1"
  • Expected: Changes to CSS/JS that are reflected on local build following refresh of webpage.

Details

  • Firefox
  • Windows 10 Home
  • Local build

Proposal & Constraints

Appropriate changes to the Docker README.md file using Yash Saravgi's suggestions.

@drakene drakene added Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Type: Bug Something isn't working. [managed] labels Feb 4, 2021
@cdrini
Copy link
Collaborator

cdrini commented Feb 4, 2021

Ah, thanks @drakene ! Yeah, that's outdated. It should be run --rm instead of exec. I opened a PR #4541. Can you test the instructions and confirm they now work?

@drakene
Copy link
Contributor Author

drakene commented Feb 4, 2021

Using the branch you created, the instructions appear to work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Type: Bug Something isn't working. [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants