Add CI check to ensure examples are documented in README #19371
Conversation
github-actions
bot
added
the
development-process
High-Level OverviewThis PR introduces a bash script to ensure all example groups in
Adds a CI step This ensures README stays in sync with example groups. |
Jefffrey
left a comment
Jefffrey
left a comment
There was a problem hiding this comment.
Minor note: the issue + PR title mention auto generating the README but this PR seems to only check that example (groups) exist in the README; is this intended?
| EXAMPLES_DIR="datafusion-examples/examples" | ||
| README="datafusion-examples/README.md" | ||
|
|
||
| SKIP_LIST=("ffi") |
There was a problem hiding this comment.
What's the reason for skipping ffi?
There was a problem hiding this comment.
ffi is skipped because it wasn’t part of the recent example consolidation work. It doesn’t follow the new example grouping and execution pattern, and therefore isn’t represented in the README using the new structure. Removing it from the check avoids false failures for a group that isn’t aligned with the documented example format.
There was a problem hiding this comment.
Can we leave a small comment next to this skip list explaining this, for future reference?
There was a problem hiding this comment.
Good point, let's do it
| # collect folder names | ||
| folders=$(find "$EXAMPLES_DIR" -mindepth 1 -maxdepth 1 -type d -exec basename {} \;) | ||
|
|
||
| # collect group names from README headers | ||
| groups=$(grep "^### Group:" "$README" | sed -E 's/^### Group: `([^`]+)`.*/\1/') |
There was a problem hiding this comment.
We only check at the group granularity? So if we add a new example to an existing group this check can miss that?
There was a problem hiding this comment.
Good point. Yes, this initial check operates at the group granularity, not at the individual example level. The idea was to introduce a lightweight, easy-to-maintain first layer of validation that ensures every group is represented in the README.
Individual examples within a group are expected to follow the existing documentation pattern. This script doesn’t enforce that yet, but it establishes a foundation we can extend in future CI improvements if needed.
There was a problem hiding this comment.
Good catch -- the current PR title isn’t fully aligned with what the PR delivers. This change focuses only on validating that example groups are documented, not on auto-generating the README. If auto-generation is considered valuable, that can certainly be explored as a follow-up PR.
There was a problem hiding this comment.
Given these reasons, the PR title should be altered and prefereably we don't close the issue, as it seems theres still some future work to be done
|
Added comment about skipping |
cj-zhukov
changed the title
|
Updated the title of the PR to one that matches the current scope |
|
Thanks @cj-zhukov |
2 participants
…#19294)
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?