Closed jmcgnh closed 4 years ago
LinuxServer-CI
commented
4 years ago I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/thelounge/4.1.0-pkg-384f17ef-pr-30/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/thelounge/4.1.0-pkg-384f17ef-pr-30/shellcheck-result.xml
aptalca
commented
4 years ago Thanks for the PR. I don't use thelounge so I can't confirm the info but the instructions read clear. One change I would request is wrapping the sample url in backticks because docker hub sometimes interprets those brackets differently and hides them http://<hostip>:9000
Thanks
jmcgnh
commented
4 years ago All requested changes have been made.
LinuxServer-CI
commented
4 years ago I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/thelounge/4.1.0-pkg-384f17ef-pr-30/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/thelounge/4.1.0-pkg-384f17ef-pr-30/shellcheck-result.xml
We welcome all PR’s though this doesn’t guarantee it will be accepted.
Description:
When I tried to use this container I ran into some difficulties. I am suggesting some improvements for the application setup block that correct some outright errors and supply a bit more guidance.
Error #1: the configuration file name is config.js not config.json
Error #2: the application is not set up for https access, so http must be used. While it is possible to set up theLounge to use https internally, there is guidance that it may make more sense to set up a reverse proxy.
Warning #1: when using the add command in the original readme, thelounge makes complaints about running as root. It will be less confusing for a user if they are given a more complete command that prevents these warnings.
Warning #2: the original language is rather confusing about where the password is set and makes it seem that one is asked to set a password in the webinterface.
The rest of my changes are intended to guide the flow of setup in a way that is easy for the user to follow. I hope I have done this without getting into the weeds of too much extraneous detail.
Benefits of this PR and context:
Frankly, only a person already familiar with thelounge as an application would be likely to make it past the errors in the current setup instructions. I'm hoping that this PR will allow more people to successfully use this container, since it seems to be about fifty-fifty whether searching for thelounge and docker will yield this container or the "official one" from thelounge.chat.
How Has This Been Tested?
I've pasted my text block into the current README.md to verify that I don't have any glaring Markdown errors.
I've set up theLounge many times, but usually not as a container. Each of the steps I've given here has been tested against the latest version of the container running on a RancherOS instance using the provided docker_compose.yml file with only the modifications needed for PUID, PGID, TZ, and config mapping.
Source / References: