Skip to content

Two updates to the readme - #23

Open
a-rahimi wants to merge 2 commits into
samyk:masterfrom
a-rahimi:master
Open

a-rahimi wants to merge 2 commits into
samyk:masterfrom
a-rahimi:master

Conversation

@a-rahimi

Copy link
Copy Markdown

There are two commits in this PR. I'm not sure if either are good enough for you, but I figured I'd try to contribute since I love this package.

  1. Use ssh instead of ncat to forward ports. This obviates the need to install a package on OSX since ssh is installed out of the box. This is how I set up my system and it seems to work reliably.

  2. Driver no longer launches from contrab. Update the readme to explain how to disable the driver, instead of explaining how to install it manually. Installing it manually is too complicated. Maybe a better approach is for easel-driver.sh to always install the EaselDriver.service, and not give the user the option to skip that step? Though I don't quite like that for systems that don't have system.

alirahimi and others added 2 commits November 15, 2022 18:15
This obviates the need to install a package on OSX since ssh is installed out of the box.
Update the readme to explain how to disable the driver, instead of
explaining how to install it manually. Installing it manually is too
complicated.

Maybe a better approach is for easel-driver.sh to always install the
EaselDriver.service, and not give the user the option to skip that step?
@samyk

samyk commented Nov 15, 2022

Copy link
Copy Markdown
Owner

Thanks! Can you please update the README to also include how to install the startup service if they didn't on initial install? This is in the case a user installs the driver, does not install the start up service, then realize they would like to add it, and will be happy to accept the PR.

@a-rahimi

Copy link
Copy Markdown
Author

I was planning to do that, but i'm afraid the instructions would be super complicated. As it stands, the install script generates two files using here-documents. The user would have to re-write these two files by hand if they decline to install them initially.

One idea is to generate a sterner warning to the user when we present them with the choice: "Your system seems to support systemd. Do you want to install the startup files? This is highly recommended. If you don't install them now and change your mind later, you can re-run this script." But if we detect the system does not have systemd, we'd say something like "Your system does not seem to have systemd. Would you still like to install as a systemd service? This is not recommended."

@samyk

samyk commented Jan 2, 2023

Copy link
Copy Markdown
Owner

I like the auto-detection route. Would try to avoid anything stern as I know I'm not going to read a lot of warnings myself when installing things :) so whatever allows the user to easily recover and ideally through the same steps...maybe we can add auto-detection of the same installed version and then re-running can ask to do a few things like "re-install, install service, uninstall service".

I'd also remove the ssh changes for now unless there's a way that doesn't require the user to do anything additional. The goal of this tool is to make it as easy as possible for someone to get working -- there's plenty that can be improved but switching from ncat to ssh then requires a password that the user may not know so complicates the setup, but if there could be keys auto-generated and added to an authorized_keys type deal, I'd go for that since the user can just copy and paste to get it working and gets the benefit of the encrypted communication.

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.

3 participants