Recursive download flag/general improvements in handling downloads #403
Conversation
This file contains 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
primeos-work
requested changes
Aug 13, 2024
There was a problem hiding this comment.
I reviewed this commit by commit so the comments might be a bit irritating since you've switched to another approach in a later commit.
Nitpick regarding the "Move XYZ into a own function" commit messages: I'd use "dedicated function" instead.
TODO: I still have to review the last commit.
ammernico
force-pushed
the
download
branch
3 times, most recently
from
0438b72
to
aaa7fd1
Compare
christophprokop
added
the
prerelease
I found the show_message a bit misleading since indicatif has the same method name for setting a real message. The method on the wrapper was just showing the progress. https://docs.rs/indicatif/0.17.8/indicatif/struct.ProgressBar.html#method.set_message Signed-off-by: Nico Steinle <[email protected]>
Refactor source download logic into a separate function for better code organization. Signed-off-by: Nico Steinle <[email protected]>
This refactor improves the maintainability and clarity of the download function. Signed-off-by: Nico Steinle <[email protected]>
- Track and display download results in an ASCII table - Make `source already exists` not an error, just a result type - Introduced a `--recursive` flag to the `download` subcommand to enable downloading of both the main packages and all their dependencies. - Added `--image` and `--env` options to specify the Docker image and environment variables, ensuring the correct dependency tree is resolved based on the build environment. Took this approach from the tree-of command. - Used a `HashSet` to avoid duplicate processing of packages and their dependencies. - Only check sources where the download was successful, skipped or forced. Use the `verify_impl` to verify the sources. Calling `super::verify` wasn't ideal since it could break on different CLI arguments. This update improves clarity and tracking of the download outcomes and allows for more comprehensive package management by ensuring that all necessary dependencies are downloaded alongside the requested packages. Signed-off-by: Nico Steinle <[email protected]>
ammernico
removed
the
prerelease
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.
No description provided.