Skip to content

Enable license header check for Skills [skip ci]#15148

Merged
YanxuanLiu merged 5 commits into
NVIDIA:mainfrom
YanxuanLiu:issue-15112-skill-license-check
Jun 26, 2026
Merged

Enable license header check for Skills [skip ci]#15148
YanxuanLiu merged 5 commits into
NVIDIA:mainfrom
YanxuanLiu:issue-15112-skill-license-check

Conversation

@YanxuanLiu

@YanxuanLiu YanxuanLiu commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15112

Description

Enable license header check for Skills

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
@YanxuanLiu YanxuanLiu requested a review from a team as a code owner June 26, 2026 02:43
@YanxuanLiu YanxuanLiu changed the title Issue 15112 skill license check Enable license header check for Skills [skip ci] Jun 26, 2026
@YanxuanLiu

Copy link
Copy Markdown
Collaborator Author

build

@YanxuanLiu YanxuanLiu added the build Related to CI / CD or cleanly building label Jun 26, 2026
@YanxuanLiu YanxuanLiu self-assigned this Jun 26, 2026
@YanxuanLiu YanxuanLiu requested review from Copilot and rishic3 June 26, 2026 02:44
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR enables the license-header-check CI workflow to cover files under the skills/ directory by adding skills/** to included_file_patterns and excluding skills/README.md, skills/.gitignore, and skills/docs/**.

  • The recursive skills/** glob correctly captures source files (.cpp, .cu, .hpp, .java, .scala, .sql, template scripts) that need copyright headers, and the exclusions handle the top-level doc files.
  • However, SKILL.md files in each subdirectory and skills/udf-gen-test/templates/scala/.mvn/jvm.config are matched by skills/** but are not excluded; these files do not carry a standard copyright comment header (the SKILL.md files use YAML front matter with a license: field rather than an SPDX comment block), so the check will fail for any PR that touches them.

Confidence Score: 3/5

The workflow change introduces a license check that will immediately fail for files that are not excluded but lack recognized copyright comment headers.

The skills/** glob matches SKILL.md files in every skill subdirectory and jvm.config. None of these files have an SPDX or Apache copyright comment header (the SKILL.md files rely on a YAML license: field, which the checker does not recognize), and none are in the exclusion list. Any PR that modifies one of those files will fail CI until they are either excluded or given proper headers.

.github/workflows/license-header-check.yml — the exclusion list needs to cover skills/**/SKILL.md and skills/udf-gen-test/templates/scala/.mvn/jvm.config (or those files need license comment headers added).

Important Files Changed

Filename Overview
.github/workflows/license-header-check.yml Adds skills/** to included patterns with exclusions for README.md, .gitignore, and docs/**; however, SKILL.md files in each subdirectory and jvm.config lack standard license comment headers and are not excluded, which will cause the check to fail on any PR touching those files.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[PR opened / synchronized] --> B[license-header-check workflow]
    B --> C{File in included_file_patterns?}
    C -- "*.yml, *.sh, *.java, *.scala, *.cpp, etc." --> D{Also in excluded_file_patterns?}
    C -- "skills/**" --> D
    D -- "skills/README.md\nskills/.gitignore\nskills/docs/**" --> E[Skip file ✓]
    D -- Not excluded --> F{Has recognized license header?}
    F -- "SPDX comment / Apache header" --> G[Pass ✓]
    F -- "YAML front matter only\n(SKILL.md files)\nor no header (jvm.config)" --> H[FAIL ✗]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[PR opened / synchronized] --> B[license-header-check workflow]
    B --> C{File in included_file_patterns?}
    C -- "*.yml, *.sh, *.java, *.scala, *.cpp, etc." --> D{Also in excluded_file_patterns?}
    C -- "skills/**" --> D
    D -- "skills/README.md\nskills/.gitignore\nskills/docs/**" --> E[Skip file ✓]
    D -- Not excluded --> F{Has recognized license header?}
    F -- "SPDX comment / Apache header" --> G[Pass ✓]
    F -- "YAML front matter only\n(SKILL.md files)\nor no header (jvm.config)" --> H[FAIL ✗]
Loading

Reviews (3): Last reviewed commit: "exclude files" | Re-trigger Greptile

Comment thread .github/workflows/license-header-check.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables the repository’s license-header-check GitHub Actions workflow to include the “skills” content, aligning CI enforcement with Issue #15112’s request to keep skill license/SPDX header years up to date when those files change.

Changes:

  • Extend the workflow’s included_file_patterns to cover files under skills/.

Comment thread .github/workflows/license-header-check.yml Outdated
@YanxuanLiu YanxuanLiu requested review from NvTimLiu and yinqingh June 26, 2026 02:49
*.java,
*.fbs
*.fbs,
skills/*

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
skills/*
skills/udf-*/**

We want everything, markdown and code, under skills to be checked, but not top-level things like skills/docs/dev or skills/README.md. We do want skills/examples/ and the upcoming skills/test/ covered, but those should already be covered under the existing extension patterns.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Only README.md and VERSIONS.md should be excluded, right?

@YanxuanLiu YanxuanLiu Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Now included all files under skills except skills/docs/** skills/README.md

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

More files will be added, e.g. see #15131 which adds pyproject.toml.

Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
@YanxuanLiu

Copy link
Copy Markdown
Collaborator Author

build

@YanxuanLiu YanxuanLiu merged commit 5537bab into NVIDIA:main Jun 26, 2026
47 checks passed
@rishic3

rishic3 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Thank you @YanxuanLiu !!

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

Labels

build Related to CI / CD or cleanly building

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a license header check for skills

5 participants