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 #76 #241

Closed
wants to merge 1 commit into from
Closed

Fix #76 #241

wants to merge 1 commit into from

Conversation

yuuyins
Copy link
Contributor

@yuuyins yuuyins commented Feb 10, 2022

If the user runs that command "from the source code folder", i.e.
getting-started, docker will bind-mount getting-started to the
container's /app, which leads to error
error Couldn't find a package.json file in "/app". It needs to bind
getting-started/app instead.
So, add /app after pwd for binding it instead, and be explicit to
the user to be on the source code's root directory getting-started
before running the command.

@StefanScherer
Copy link
Member

Thanks. Ok, this is a thing we should definitely improve in the docs. But I prefer to keep $(pwd) in it to make it work when users want to copy-paste the command. I'd like to make the cd app more prominent, maybe as an example before docker run. I see that Run the following command from the source code folder. may not be obvious enough. WDYT?

If the user runs that command "from the source code folder", i.e.
getting-started, docker will bind-mount getting-started to the
container's /app, which leads to error
error Couldn't find a package.json file in "/app". It needs to bind
getting-started/app instead.
So, add `/app` after `pwd` for binding it instead, and be explicit to
the user to be on the source code's root directory `getting-started`
before running the command.
@yuuyins
Copy link
Contributor Author

yuuyins commented Feb 12, 2022

@StefanScherer i think for didactic purposes, it would be appropriate to explicitly show in the command that we are binding app from the host. So I've added /app after pwd

-        -w /app -v "$(pwd):/app" \
+        -w /app -v "$(pwd)/app:/app" \

and made it explicit to the reader to be on the source's root directory. WDYT?

@yuuyins yuuyins mentioned this pull request Feb 12, 2022
@yuuyins
Copy link
Contributor Author

yuuyins commented Feb 12, 2022

@StefanScherer i created another pr with the example you suggested #243. feel free to pick either of the prs and close either.

@StefanScherer
Copy link
Member

Closing this in favor of your other PR.

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