-
Notifications
You must be signed in to change notification settings - Fork 9
Multiple fixes after V0.2.10 #375
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
Conversation
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
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.
Pull Request Overview
This pull request introduces documentation enhancements, improvements to the CLI commands, and workflow updates for better code quality and usability.
- Updated package installation commands in the container toolkit template
- Enhanced AWS instance creation and deletion behavior in the CLI
- Expanded documentation and added new CI workflows for Markdown linting
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/provisioner/templates/container-toolkit.go | Updated package installation command with retry logic and added packages |
| pkg/provider/aws/create.go | Added instance shutdown behavior in instance creation |
| docs/* | Added and updated documentation files for quick-start, commands, etc. |
| cmd/cli/list/list.go | Updated list command to support quiet flag with pointer receiver |
| cmd/cli/delete/delete.go | Enhanced delete command to support deletion of multiple instances |
| cmd/cli/create/create.go | Improved logging in create command for better output readability |
| README.md | Revised formatting and content for improved clarity |
| Makefile | Added mdlint target for local Markdown linting |
| .github/workflows/*.yaml | Introduced new workflow for documentation linting and updated CI workflow dependencies |
Comments suppressed due to low confidence (1)
pkg/provisioner/templates/container-toolkit.go:37
- Consider aligning the continuation line for the package installation command with consistent indentation to improve readability.
install_packages_with_retry nvidia-container-toolkit nvidia-container-toolkit-base \
| return fmt.Errorf("failed to get instance: %v", err) | ||
| } | ||
| // Process each instance ID provided as an argument | ||
| for _, instanceID := range c.Args().Slice() { |
Copilot
AI
May 29, 2025
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.
[nitpick] Consider accumulating errors during deletion of multiple instances rather than returning immediately on the first error, so that all valid deletions are attempted.
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
efbd194 to
36f0b71
Compare
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
36f0b71 to
99ec561
Compare
This pull request introduces significant enhancements to the Holodeck project, focusing on documentation, workflow improvements, and CLI feature updates. The changes include adding a new Markdown linting workflow, expanding the documentation, and improving the CLI commands for better usability and functionality.
Workflow Enhancements
docs-checkjob to the CI pipeline in.github/workflows/ci.yaml, which runs a Markdown linter to ensure documentation quality..github/workflows/docs_check.yaml, to define thedocs-checkjob. It includes steps for setting up Ruby, installing themdllinter, and running it on Markdown files.Documentation Improvements
README.mdwith better organization, quick links to documentation, and a more user-friendly structure.docs/commands/, including detailed usage, flags, examples, and error handling for each CLI command (create,delete,list,status,dryrun). [1] [2] [3] [4] [5] [6]CLI Feature Enhancements
deletecommand to support deleting multiple instances at once by accepting instance IDs as arguments instead of a single flag. Improved error handling for invalid or missing IDs. [1] [2]listcommand with a new--quietflag to display only instance IDs, making it easier to use in scripts. [1] [2] [3]createcommand to include a newline for better readability.Makefile Updates
mdlinttarget to theMakefilefor running Markdown linting locally using a Docker container. [1] [2]