Skip to content

Conversation

drwhomphd
Copy link
Contributor

Motivation

By default git submodule does not display download progress. This small feature allows users to monitor downloads to insure downloads have not stalled. This is a benefit for the larger submodules that can take some time to download.

Technical Details

This patch adds an option, --progress, to the script arguments. When passed to the python script it is added to arguments passed to the submodule command.

Test Plan

Tested running fetch sources with the --progress flag passed.

Test Result

Flag works as intended.

By default git submodule does not display download progress. This
patch adds an option, --progress, for enabling download progress
being displayed when fetch sources is downloading git submodules.
This small feature is beneficial for the download of some larger
submodules that can take some time to download.
@marbre marbre requested review from ScottTodd and marbre September 29, 2025 07:15
@ScottTodd
Copy link
Member

Sorry for the delay reviewing this. pre-commit wants a few small style changes: https://github.com/ROCm/TheRock/actions/runs/17962344001/job/51515804473?pr=1574#step:4:187. I can push a commit fixing them.

@ScottTodd
Copy link
Member

Ah actually I'm having trouble pushing to your remote... maybe something broken about my git config. Can you run pre-commit yourself, or apply the diffs from those logs?

@drwhomphd
Copy link
Contributor Author

drwhomphd commented Oct 2, 2025

Sorry for the delay in catching this...

I just told Github to merge the most recent upstream branch. I'm hoping that makes pre-commit work successfully. If not I will manually make the changes. Upon looking at the 'diff' the best I could tell it may be a quote issue. I will post an additional update as soon as either the pre-commit works or I have manually made the pre-commit changes.

@ScottTodd
Copy link
Member

Yep whitespace and single quotes vs double quotes: https://github.com/ROCm/TheRock/actions/runs/18206616770/job/51838746742?pr=1574#step:4:179. We have docs on how to set up pre-commit to format for you at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#pre-commit-checks

@drwhomphd
Copy link
Contributor Author

@ScottTodd Understood. Apologies for missing that on my first pass through the CONTRIBUTING doc I made. Fixing things now!

@drwhomphd
Copy link
Contributor Author

Ok, pre-commit ran successfully on the local machine so it should pass in CI! Appreciate you walking me through this commit process!

@ScottTodd
Copy link
Member

No problem, thanks for your patience and for putting up with the linter 😅. Sorry I wasn't able to push those fixes myself.

CI builds got past this new code (that should not be changing behavior by default), so merging without waiting for them to complete.

@ScottTodd ScottTodd merged commit 4722548 into ROCm:main Oct 2, 2025
7 checks passed
@github-project-automation github-project-automation bot moved this from TODO to Done in TheRock Triage Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants