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

Make poly work on linux - bare minimum. #1

Merged
merged 3 commits into from Sep 29, 2021
Merged

Conversation

tjstub
Copy link
Contributor

@tjstub tjstub commented Sep 29, 2021

This is just a quick improvement to do the bare-minimum to make poly work out of the box on linux. Namely,

  • Packages should be installed locally.
  • Some commands need to be Sudo
  • Pillow was missing from the requirements.

There are some other improvements that could be made to the install script. Suggestion for future improvements:

  1. the clone is done regardless of whether a repo exists or not.
  2. Clone Goes to the cwd; Where the user is may or may not be writeable. Try for ~/ or /tmp
  3. Consider a trap for ensuring cleanup on the bash script.
  4. Probably shouldn't go to /usr/bin/ for the app. Mayhaps /usr/local/bin or ~/.local/bin? Can check path to decide.
  5. Why does it matter if poly exists? what if I want to update it? Overwrite that guy!
  6. Rather than -f /usr/bin/poly, consider command or which to discover it
  7. Consider enabling bash "safe mode"
  8. Mark script with /usr/bin/env bash
  9. Consider removing absolute path commands. Could cause issues on other systems.

If you want, I can make a ticket for these, if you agree, I just wanted to drop this by while it was in my head. Good work, it's pretty cool!

@tjstub tjstub changed the title Fixed the bare minimum to make poly work. Make poly work on linux - bare minimum. Sep 29, 2021
poly Outdated Show resolved Hide resolved
@3digitdev 3digitdev merged commit a6e19bc into 3digitdev:master Sep 29, 2021
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.

None yet

2 participants