-
Notifications
You must be signed in to change notification settings - Fork 242
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
Docker setup for pyrdp #66
Conversation
The certificate cloner and the Man-in-the-middle features are both working in the docker.
Pyrdp-player is now working in a container
pyRDP can now be use with docker compose
The user in the container is no longer "root", but "developer". This simplifies the use of the pyrdp-player because the "xhost +local:docker" command is no longer required.
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.
Good but some more work required. For the documentation, I guess I wasn't clear: I would like the README to cover how to use docker (not docker-compose) and the docker-compose is more as an example (no need to talk about it in the README).
General improvements have been made in the documentation and some points have been clarified
The Dockerfile now takes advantage of caching. Plus, some typos have been corrected in the doc. The user in the container is now "pyrdp" instead of "developer" to simplify.
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.
Another round of review done. There are only very small changes to do left. The documentation is very good by the way 👍
The assignment of QT_X11_NO_MITSHM is now done with the run command and explained in the README. This prevents the user from modifying the Dockerfile.
QT_X11_NO_MITSHM=1 added to the environment section
sudo was removed from all the docker commands, QT_X11_NO_MITSHM=1 was added to the run options and some minor typos were corrected in the README
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.
LGTM!
-The run command example in the README is more explicit. -The build command now uses the image name.
@xshill can you take another look when you have a minute? |
Corrections of the README so the installation commands are more specific.
No description provided.