Skip to content

Avoid calling 'git lfs install' as CI already performs this#833

Merged
rapids-bot[bot] merged 1 commit intoNVIDIA:developfrom
dagardner-nv:david-git-lfs-install-error
Sep 22, 2025
Merged

Avoid calling 'git lfs install' as CI already performs this#833
rapids-bot[bot] merged 1 commit intoNVIDIA:developfrom
dagardner-nv:david-git-lfs-install-error

Conversation

@dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Sep 22, 2025

Description

  • Avoids a CI error about the LFS hooks already being installed.

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

  • Chores
    • Streamlined CI handling of large file assets by simplifying LFS setup, reducing redundant steps and potential points of failure in certain environments. This should yield faster, more reliable pipeline runs with cleaner logs during fetch/pull stages.
    • No impact on product features or user workflows; changes are limited to build infrastructure to improve consistency and maintainability of continuous integration processes.

Signed-off-by: David Gardner <dagardner@nvidia.com>
@dagardner-nv dagardner-nv self-assigned this Sep 22, 2025
@dagardner-nv dagardner-nv requested a review from a team as a code owner September 22, 2025 18:14
@dagardner-nv dagardner-nv added bug Something isn't working non-breaking Non-breaking change labels Sep 22, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

Removed the git lfs install step from get_lfs_files in ci/scripts/common.sh. The function now directly runs git lfs fetch and git lfs pull when not using host git, changing the control flow within the CI script.

Changes

Cohort / File(s) Summary
CI LFS flow adjustment
ci/scripts/common.sh
In get_lfs_files, dropped explicit git lfs install invocation; now calls git lfs fetch and git lfs pull directly when host git is not used.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor CI as CI Runner
  participant SH as common.sh:get_lfs_files
  participant G as git (LFS)

  rect rgb(240,245,255)
  note over CI: Previous flow
  CI->>SH: invoke get_lfs_files
  SH->>G: git lfs install
  SH->>G: git lfs fetch
  SH->>G: git lfs pull
  SH-->>CI: done
  end
Loading
sequenceDiagram
  autonumber
  actor CI as CI Runner
  participant SH as common.sh:get_lfs_files
  participant G as git (LFS)

  rect rgb(240,255,240)
  note over CI: New flow
  CI->>SH: invoke get_lfs_files
  SH->>G: git lfs fetch
  SH->>G: git lfs pull
  SH-->>CI: done
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately summarizes the main change (removing a redundant 'git lfs install' call), is written in the imperative mood, and is concise and under the 72-character limit; it aligns with the PR objectives and the raw_summary of changes.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fea8dcd and f4b20ce.

📒 Files selected for processing (1)
  • ci/scripts/common.sh (0 hunks)
💤 Files with no reviewable changes (1)
  • ci/scripts/common.sh

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit e3ce04a into NVIDIA:develop Sep 22, 2025
17 checks passed
@dagardner-nv dagardner-nv deleted the david-git-lfs-install-error branch September 22, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants