Skip to content

Refactor Shell. Remove ShellOut.#34

Merged
albertodebortoli merged 20 commits intomasterfrom
remove-shellout
Mar 27, 2023
Merged

Refactor Shell. Remove ShellOut.#34
albertodebortoli merged 20 commits intomasterfrom
remove-shellout

Conversation

@albertodebortoli
Copy link
Member

This PR initially aimed to remove ShellOut in favour of Apple's TSCBasic but I couldn't wrap my head around it without first reworking Shell.

  • Refactor Shell to have a clearer interface that only wraps the underlying shell from TSCBasic (e.g. avoid duplication of Process)
  • Remove logger from Shell (it has stdout and stderr and best to leave print statements at the call sites to avoid mixing responsibilities)
  • Move commands out of Shell to Shell+Commands to have clear responsibilities
  • Remove which and downloadResource commands as they are unused. They seem to be leftovers from Added StellarEnv to manage and communicate with stellar installed versions #24.
  • Split into more files: ShellError and ProcessResult+Throwing
  • Remove usage of sudo as not needed (/usr/local/bin is writable without sudo) and wasn't working (it exits the program as needs user input)
  • Reorganise some files on disk
  • Remove ShellOut in favour of Apple's TSCBasic

@albertodebortoli albertodebortoli added this to the 0.1.0 milestone Mar 22, 2023
@albertodebortoli
Copy link
Member Author

Ok chaps, sorted by reintroducing @malcommac's code :)

[Stellar] Expanding the archive…
[Stellar] Installing at /usr/local/bin/stellarenv…
rm: /usr/local/bin/stellarenv: Permission denied
Password:
mv: rename /var/folders/7s/4wmmddq506gbsdvlr53sd0lh0000gn/T/com.stellarenv-D36BF29A-8BE8-47F5-93AC-C69008AA72C1/StellarEnv to /usr/local/bin/stellarenv: Permission denied
[Stellar] StellarEnv version 0.0.8 installed
[Stellar] Stellar version 0.0.8 installed

I tried on a different mac where my user is in the wheel group (see https://superuser.com/questions/20420/what-is-the-difference-between-the-default-groups-on-mac-os-x). I'm still not clear on the details of user management in unix but frankly I'm not interested now. ChatGPT will teach me anyway next time I need this :)

I've also added the shared schemes for StellarEnv.

@albertodebortoli albertodebortoli added the enhancement New feature or request label Mar 26, 2023
Copy link
Contributor

@malcommac malcommac left a comment

Choose a reason for hiding this comment

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

👍

@albertodebortoli albertodebortoli merged commit 884d541 into master Mar 27, 2023
@albertodebortoli albertodebortoli deleted the remove-shellout branch March 27, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants