-
Notifications
You must be signed in to change notification settings - Fork 95
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
Introduce a mode so one call install from git #690
Conversation
If you have git clone'd the project, you now have the option of doing: ./install.sh -l And it will install the version from the git repo. Signed-off-by: Eric Curtin <[email protected]>
Reviewer's Guide by SourceryThis pull request introduces a new local install mode that allows users to install the application directly from a git repository, instead of downloading it from a remote server. This is achieved by adding a new '-l' flag to the install script. Sequence diagram for local vs remote installationsequenceDiagram
participant User
participant Script as Install Script
participant Local as Local Git Repo
participant Remote as Remote Server
User->>Script: Run install.sh
alt With -l flag
Script->>Local: Copy files from local repo
Local-->>Script: Return local files
else Without -l flag
Script->>Remote: Request files via HTTPS
Remote-->>Script: Return remote files
end
Script->>Script: Install files to system
Script-->>User: Installation complete
Flow diagram for the updated installation processgraph TD
Start[Start Installation] --> ParseArgs[Parse Command Line Arguments]
ParseArgs --> CheckFlag{Check -l flag}
CheckFlag -->|Local Install| GitRepo[Use Git Repository Files]
CheckFlag -->|Remote Install| RemoteServer[Download from Remote Server]
GitRepo --> CheckPlatform[Check Platform Requirements]
RemoteServer --> CheckPlatform
CheckPlatform --> InstallDeps[Install Dependencies]
InstallDeps --> SetupFiles[Setup RamaLama Files]
SetupFiles --> Cleanup[Cleanup Temporary Files]
Cleanup --> End[End Installation]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @ericcurtin - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider keeping the uppercase TMP variable name for consistency with Unix conventions and to avoid breaking existing scripts that might reference it.
- Error handling should be made consistent across functions - some return explicit error codes while others don't return anything.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
return 2 | ||
fi | ||
if [ "$os" = "Darwin" ]; then | ||
install_mac_dependencies |
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.
issue (bug_risk): The return value from install_mac_dependencies() should be checked
install_mac_dependencies() returns different error codes, but these aren't being handled. This could lead to silent failures during installation.
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.
We use set -e in this script so I don't think there's a need
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.
Thank you for your feedback, we will generate fewer comments like this in the future.
LGTM |
If you have git clone'd the project, you now have the option of doing:
./install.sh -l
And it will install the version from the git repo.
Summary by Sourcery
New Features:
-l
flag to theinstall.sh
script to install the CLI from a local git clone.