Skip to content

Conversation

@cj-zhukov
Copy link
Contributor

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?

@cj-zhukov
Copy link
Contributor Author

High-level overview

This PR follows up on review feedback and focuses on small structural and correctness improvements, without changing any user-facing behavior.

Specifically, this PR:

  • Moves the ABBREVIATIONS constant from render to model to eliminate an unnecessary circular dependency between modules.
  • Improves discover_example_groups to better handle edge cases, such as empty example roots or directories named main.rs. The function now fails fast with a clear error when no valid example groups are found.
  • Adds additional unit tests for the discovery logic to cover these edge cases and make the behavior explicit and well-documented.

@cj-zhukov
Copy link
Contributor Author

@martin-g I’ve addressed your comments in this PR by refactoring the module structure, fixing the example group discovery edge cases, and adding tests to cover them.
Would appreciate another look when you have time - happy to iterate further on naming or structure if needed.

@cj-zhukov
Copy link
Contributor Author

@comphead @Jefffrey since you helped me with previous PRs related to example docs generation, it would be great if you could take a look at this one as well. Your feedback or any improvements would be much appreciated.

}

if groups.is_empty() {
return Err(DataFusionError::Execution(format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return Err(DataFusionError::Execution(format!(
return exec_err!(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I'm going to update to use exec_err! for consistency with the rest of code.

@cj-zhukov
Copy link
Contributor Author

I updated the code to use exec_err! in places where it fits naturally, did a small refactor without changing behavior, and aligned imports with the conventions used elsewhere in datafusion-examples utilities

Copy link
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 @cj-zhukov and @Jefffrey

@comphead comphead added this pull request to the merge queue Feb 11, 2026
Merged via the queue into apache:main with commit 17416bf Feb 11, 2026
28 checks passed
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.

4 participants