Skip to content

We have now the CI ensure all doc strings remain formatted #16916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yazanmashal03
Copy link

Which issue does this PR close?

Rationale for this change

This change ensures the CI process has all docs strings remain formatted.

What changes are included in this PR?

The rustfmt.toml enables the "format_code_in_doc_comments" feature. I also added the ci file in the .github directory.

Are these changes tested?

Yes, running "cargo +nightly fmt --all -- --config format_code_in_doc_comments=true" can duplicate the effect of this solution.

Are there any user-facing changes?

No. And thank you for a first-issue. I hope it is good, and if there is anything I can add, let me know.

@github-actions github-actions bot added sql SQL Planner development-process Related to development process of DataFusion logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate common Related to common crate execution Related to the execution crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate spark labels Jul 25, 2025
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @yazanmashal03 -- this is looking like a very nice improvement

Given the fact that this feature requires cargo nightly which we don't typically use in CI here is what I suggest:

  1. Remove the changes to CI and rustfmt.toml from this PR
  2. Run the tool manually to format the examples (keep that in this PR)
  3. File a new ticket/PR with just the changes to rustfmt.toml which we can merge once the format_code_in_doc_comments feature is related in stable

@@ -17,6 +17,7 @@

edition = "2021"
max_width = 90
format_code_in_doc_comments = true # nightly-only feature
Copy link
Contributor

Choose a reason for hiding this comment

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

When I run cargo fmt now locally I see many warnings like this

(venv) andrewlamb@Andrews-MacBook-Pro-3:~/Software/datafusion$ cargo fmt
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.
Warning: can't set `format_code_in_doc_comments = true`, unstable features are only available in nightly channel.

Which I think means we have to wait until this feature is actually released in stable Rust to add it to this file

run: rustup toolchain install nightly

- name: Run rustfmt (nightly)
run: cargo +nightly fmt --all -- --config format_code_in_doc_comments=true
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @yazanmashal03

I am not sure what this check is doing - it seems to just run the fmt rather than check its output, for example

@alamb alamb removed the spark label Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Related to common crate core Core DataFusion crate datasource Changes to the datasource crate development-process Related to development process of DataFusion execution Related to the execution crate functions Changes to functions implementation logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate proto Related to proto crate sql SQL Planner sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chore: format documentation examples
2 participants