Skip to content

docs: prefer behavior-named parameters over Spark versions - #6776

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:andygrove/functional-parameters-guide
Open

andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:andygrove/functional-parameters-guide

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A. Follows the review discussion on #6740.

Rationale for this change

In #6740 the question came up of why the new protobuf flag is named wrap_second_millisecond_overflow rather than spark_420_plus. The reasoning is worth writing down so contributors and reviewers apply it consistently:

  • Forks that backport fixes from newer open-source Spark can set a behavior flag from their own shim.
  • Spark sometimes changes behavior in patch or minor releases, so a version number is a poor proxy for behavior.
  • Native code stays free of Spark version logic.

What changes are included in this PR?

  • adding_a_new_expression.md: new "Name the behavior, not the Spark version" subsection under "API Differences Between Spark Versions", including guidance to write comments as "Spark 4.2 and later" when later releases inherit the behavior.
  • review-comet-pr and review-comet-expression-pr skills: reviewers flag parameters, proto fields, and comments named for a Spark version instead of the behavior.
  • implement-comet-expression and wire-datafusion-function skills: point authors to the new guide section.

How are these changes tested?

Documentation only. Checked with prettier.

When behavior varies between Spark versions, resolve the version in Scala
and pass native code a parameter that describes the behavior. Add the
guidance to the contributor guide and the expression skills.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant