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

update documentation #14

Merged
merged 4 commits into from
Apr 4, 2024
Merged

update documentation #14

merged 4 commits into from
Apr 4, 2024

Conversation

koebel
Copy link
Contributor

@koebel koebel commented Apr 4, 2024

I've made some minor updates in the documentation and added more links to platforms where one can download 3D models.

Please check in particular if the url for accessing the app when running it with Docker is correct.

@koebel koebel mentioned this pull request Apr 4, 2024
README.md Outdated
Comment on lines 22 to 23
```
You can access the app at https://host.docker.internal:9200
Copy link
Owner

@saw-jan saw-jan Apr 4, 2024

Choose a reason for hiding this comment

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

this would be wrong here because this section doesn't say about serving ocis.
What can be done here is, remove docker run example and only say which port does the image exposes. like this:

- Run the extension container with:
- ```bash
- docker run --rm -p3000:80 sawjan/web-app-3dmodel-viewer
- ```
+ Exposed port: `80`

This should prevent running docker run command.
Or, we can move this whole section to the buttom after Build and Run

Copy link
Contributor Author

@koebel koebel Apr 4, 2024

Choose a reason for hiding this comment

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

I like that idea of changing the order. I think that makes it clearer. And I guess for those who use Docker frequently, I guess the would see from the command that it is exposed at port 80

@saw-jan saw-jan merged commit 995cdb7 into saw-jan:master Apr 4, 2024
1 check passed
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