Skip to content

Conversation

@YanxuanLiu
Copy link
Collaborator

Add shellcheck shared action

Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
@YanxuanLiu
Copy link
Collaborator Author

Signed-off-by: Yanxuan Liu <[email protected]>
@YanxuanLiu YanxuanLiu self-assigned this Aug 4, 2025
@YanxuanLiu YanxuanLiu marked this pull request as ready for review August 4, 2025 02:30
@YanxuanLiu YanxuanLiu requested a review from a team as a code owner August 4, 2025 02:30
@pxLi pxLi requested a review from Copilot August 4, 2025 02:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new reusable ShellCheck GitHub Action and refactors the existing workflow to use it. The action provides a configurable shell script linting solution with predefined exclusions for common ShellCheck warnings.

Key changes:

  • Creates a new shared ShellCheck action with configurable excluded error codes
  • Simplifies the existing shell-check workflow by using the new shared action
  • Removes unnecessary fetch-depth configuration from the workflow

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
shell-check/action.yml New shared action that configures ShellCheck with predefined exclusions and allows additional user-specified exclusions
.github/workflows/shell-check.yml Simplified workflow that now uses the new shared action instead of directly calling ludeeus/action-shellcheck
Comments suppressed due to low confidence (1)

echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 10 ))" >> $GITHUB_ENV
- name: Checkout code
uses: actions/checkout@v4
Copy link
Member

@pxLi pxLi Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the checkout would be better to not be put at the caller side, as mentioned before, otherwise you will need to update all refs later in multiple places in the future

Copy link
Member

@pxLi pxLi Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW license header check has the same issue, please help fix all refs when available

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. will file another PR for a checkout wrapper

NvTimLiu
NvTimLiu previously approved these changes Aug 4, 2025
pxLi
pxLi previously approved these changes Aug 5, 2025
Copy link
Member

@pxLi pxLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help do some post-merge verification after force merge thanks

Signed-off-by: Yanxuan Liu <[email protected]>
pxLi
pxLi previously approved these changes Aug 5, 2025
@YanxuanLiu
Copy link
Collaborator Author

@YanxuanLiu YanxuanLiu merged commit 7b45825 into NVIDIA:main Aug 5, 2025
4 of 6 checks passed
@YanxuanLiu
Copy link
Collaborator Author

test PR: #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants