Repository navigation
[no-ci] Mention mostly the ability to automatically update submodules - #3105
Conversation
Expand docs slightly to mention that you can set up your clone to automatically keep submodules up to date.
seberg
added
documentation
📝 Summary
Priority: ⬇️ Low Change: Other Merge Risk: 🔵 Low · up to
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
78a91f92-c74c-4dcc-9f43-4037f0a89fe7
📒 Files selected for processing: 1
CONTRIBUTING.md
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
rwgk
left a comment
rwgk
left a comment
There was a problem hiding this comment.
I went ahead and added a commit to simplify/polish the new sections. The changes take the coderabbit suggestions into account.
rwgk
changed the title
|
Closing / re-opening to re-trigger GHA after adding [no-ci]. |
|
|
||
| This setting can also make `git push` recurse into submodules. To keep | ||
| `git push` limited to this repository, also set this once per clone: | ||
|
|
||
| ```console | ||
| $ git config --local push.recurseSubmodules no | ||
| ``` |
There was a problem hiding this comment.
| This setting can also make `git push` recurse into submodules. To keep | |
| `git push` limited to this repository, also set this once per clone: | |
| ```console | |
| $ git config --local push.recurseSubmodules no | |
| ``` |
I delete this again, but wait a bit so you have a chance to disagree if you like.
My understanding is: This setting is irrelevant unless you develope CCCL in-tree. That seems too niche information for CONTRIBUTING.md, IMO, even if it makes sense as such.
There was a problem hiding this comment.
After looking deeper: I agree.
My conversation with codex:
Me: I wonder: won't that let people run into a trap? But then again, I'm not really sure what the situation will be. How could people get into the situation to unintentionally push to a submodule? They'd have to intentionally create a commit first? They'd also have to set up a tracking branch?
codex:
Fresh view: Sebastian removed the push warning in fb4a9dd. After checking the details, I think that’s reasonable. My earlier recommendation overstated the risk for ordinary contributors.
The key requirement is a CUDA Python commit being pushed that records an unpublished CCCL commit. Recursive push then attempts to publish that dependency first. Ordinary updates to already-published CCCL commits don’t require this. Git documentation
I verified these cases with isolated local repositories:
- Editing or staging CCCL files does not publish them.
- Creating a CCCL commit does not trigger a push if CUDA Python still records the old CCCL commit.
- Committing the changed CCCL reference in CUDA Python can trigger the recursive push.
On your specific questions:
- Must they create a CCCL commit? Usually, but they needn’t author it themselves; they could obtain someone else’s commit and record it in CUDA Python.
- Must they configure a tracking branch? No. I successfully pushed both repositories using
git push origin mainwith no tracking configured for the submodule branch. A suitable local branch and explicit refspec sufficed. - What about the usual detached HEAD? In my test, an unpublished detached commit without a suitable branch caused the parent push to fail rather than publish it.
So the possible surprise is automatic publication after someone has recorded an unpublished dependency commit—not accidentally publishing routine local edits. I’d accept Sebastian’s simpler instructions.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · important: Initialize nested submodules recursively. · CONTRIBUTING.md:123-134
CONTRIBUTING.md:123-134
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winimportant: Initialize nested submodules recursively.
When a pulled revision adds a submodule below an existing submodule,
git submodule update --initdoes not descend into that existing submodule. The nested submodule remains uninitialized, despite the instruction claiming to initialize any newly added submodules. Add--recursive; this is a recursion-depth issue, not the top-level path-specific initialization issue.Suggested fix
-$ git pull --recurse-submodules && git submodule update --init +$ git pull --recurse-submodules && git submodule update --init --recursive
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
95645afb-7921-41e9-99b8-9834b90894f2
📒 Files selected for processing: 1
CONTRIBUTING.md
💤 Files with no reviewable changes: 1
- CONTRIBUTING.md
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
2 participants
Expand docs slightly to mention that you can set up your clone to automatically keep submodules up to date.
@rwgk is this roughly what you are looking for?