Skip to content

docs: add implement-comet-expression Claude skill#4158

Merged
andygrove merged 3 commits intoapache:mainfrom
andygrove:add-implement-expression-skill
May 1, 2026
Merged

docs: add implement-comet-expression Claude skill#4158
andygrove merged 3 commits intoapache:mainfrom
andygrove:add-implement-expression-skill

Conversation

@andygrove
Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

We already ship two Comet-specific Claude skills under .claude/skills/: audit-comet-expression for auditing existing expressions and review-comet-pr for reviewing PRs. There is no companion skill for the most common contributor task, which is implementing a brand new expression. This PR adds one.

The skill is intentionally a thin wrapper around docs/source/contributor-guide/adding_a_new_expression.md rather than a duplicate. It encodes the workflow conventions that are not captured in the contributor guide on their own:

  • Always read the latest Spark master first via a shallow clone, so the canonical behavior comes from upstream rather than from whichever Spark version the contributor happens to have checked out.
  • Run the existing audit-comet-expression skill against the initial implementation to drive a structured test-coverage iteration loop.

What changes are included in this PR?

Adds .claude/skills/implement-comet-expression/SKILL.md, a short skill that:

  1. Points the implementer at the relevant contributor-guide pages.
  2. Walks through cloning Spark master and locating the expression class and tests.
  3. Summarizes the implementation steps (Scala serde, registration, optional scalarFunctionExprToProtoWithReturnType, native scalar function, initial Comet SQL Test).
  4. Hands off to audit-comet-expression for gap analysis.
  5. Iterates on audit-recommended tests until the user is satisfied.
  6. Finishes with the standard pre-PR make format and cargo clippy checks.

How are these changes tested?

This is a documentation-only change, so no automated tests are added. The skill was reviewed for accuracy against the current contributor guide and the behavior of the existing audit-comet-expression skill.

Copy link
Copy Markdown
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @andygrove I think we can merge it now and test

@andygrove andygrove merged commit dd05146 into apache:main May 1, 2026
5 checks passed
@andygrove andygrove deleted the add-implement-expression-skill branch May 1, 2026 00:52
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.

2 participants