Multiple fixes after V0.2.10#375
Merged
ArangoGutierrez merged 4 commits intoNVIDIA:mainfrom May 29, 2025
Merged
Conversation
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
[nitpick] Consider accumulating errors during deletion of multiple instances rather than returning immediately on the first error, so that all valid deletions are attempted.
efbd194 to
36f0b71
Compare
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
36f0b71 to
99ec561
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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]