-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add missing files to install.sh script #524
Conversation
file.py , http_client.py , etc. Signed-off-by: Eric Curtin <[email protected]>
Reviewer's Guide by SourceryThis pull request adds missing files to the install.sh script. The changes ensure that necessary files are included in the installation process. This addresses the issue of missing files in the previous version of the script. Sequence diagram for updated installation processsequenceDiagram
participant User as User
participant Install as install.sh
participant Server as Remote Server
User->>Install: Execute install.sh
Install->>Install: setup_ramalama()
loop For each Python file
Install->>Server: Download file
Note right of Install: Now includes additional files:
Note right of Install: file.py, http_client.py,
Note right of Install: url.py, annotations.py
Server-->>Install: Return file content
Install->>Install: Install file
end
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:
- Please expand the PR description to explain why these files are being added and what functionality they provide. This helps reviewers and future maintainers understand the context of the change.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 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.
local python_files=("cli.py" "huggingface.py" "model.py" "ollama.py" "common.py" "__init__.py" \ | ||
"quadlet.py" "kube.py" "oci.py" "version.py" "shortnames.py" "toml_parser.py") | ||
|
||
local python_files=("cli.py" "huggingface.py" "model.py" "ollama.py" \ |
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.
Wouldn't it be easier to just use a GLOB for this?
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.
The way this installer works is it pulls down install.sh via curl and then the various other files it needs via curl. We could maybe pull down the whole project as a zip/tarball , etc. and then do a glob, but we would pull a lot of unnecessary things also.
But yeah I've also been thinking maybe we can look into somehow making this smarter rather than listing all the files.
Merging but I think we should open a PR to just grab the py files in the ramalama/*py. |
file.py , http_client.py , etc.
Summary by Sourcery
Build:
install.sh
script.