Skip to content

dev: Add CI doc prettier check to local rust_lint.sh #19254

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.

Sign up for GitHub

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

Merged
alamb merged 3 commits into from
Dec 11, 2025
Merged

dev: Add CI doc prettier check to local rust_lint.sh #19254

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32762e3
add doc prettier check to `rust_lint.sh`
2010YOUY01 Dec 10, 2025
f8296dd
revert prettier check violation
2010YOUY01 Dec 10, 2025
573fd64
review: update the script with --write option
2010YOUY01 Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions .github/workflows/dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,8 @@ jobs:
with:
node-version: "20"
- name: Prettier check
run: |
# if you encounter error, rerun the command below and commit the changes
#
# ignore subproject CHANGELOG.md because they are machine generated
npx prettier@2.7.1 --write \
'{datafusion,datafusion-cli,datafusion-examples,dev,docs}/**/*.md' \
'!datafusion/CHANGELOG.md' \
README.md \
CONTRIBUTING.md
git diff --exit-code
# if you encounter error, see instructions inside the script

@alamb alamb Dec 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

run: ci/scripts/doc_prettier_check.sh

typos:
name: Spell Check with Typos
Expand All @@ -72,4 +64,4 @@ jobs:
- name: Install typos-cli
run: cargo install typos-cli --locked --version 1.37.0
- name: Run typos check
run: ci/scripts/typos_check.sh
run: ci/scripts/typos_check.sh
43 changes: 43 additions & 0 deletions ci/scripts/doc_prettier_check.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

SCRIPT_PATH="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")"
echo "$SCRIPT_PATH: Checking document format with prettier"

# Ensure `npx` is available
if ! command -v npx >/dev/null 2>&1; then
echo "npx is required to run the prettier check. Install Node.js (e.g., brew install node) and re-run." >&2
exit 1
fi

# if you encounter error, change '--check' to '--write' in the below command, and

@alamb alamb Dec 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tested this out locally and it worked great for me:

$ ./ci/scripts/doc_prettier_check.sh
/Users/andrewlamb/Software/datafusion/ci/scripts/doc_prettier_check.sh: Checking document format with prettier
Checking formatting...
[warn] docs/source/library-user-guide/upgrading.md
[warn] Code style issues found in the above file. Forgot to run Prettier?
./ci/scripts/doc_prettier_check.sh: line 41: prettier: command not found
Prettier check failed. To fix, rerun  with --write (change --check to --write in the script), commit the formatted files, and re-run the check.

I think it would be even nicer / better if it could also make the changes as well (so people don't have to do a second step to update the prettier docs)

For example, if the check fails, maybe it could automatically re-run prettier with --write

Or maybe the script could accept a --write command to get it to write?

$ ./ci/scripts/doc_prettier_check.sh --write

@2010YOUY01 2010YOUY01 Dec 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point! Updated in 573fd64

Now for check failures, the error message look like

yongting@Yongtings-MacBook-Pro-2 ~/C/datafusion (doc-prettier-script *=) [1]> ./ci/scripts/doc_prettier_check.sh
/Users/yongting/Code/datafusion/ci/scripts/doc_prettier_check.sh: Checking documents with prettier
Checking formatting...
[warn] docs/source/user-guide/metrics.md
[warn] Code style issues found in the above file. Forgot to run Prettier?
Prettier check failed. Re-run with --write (e.g., ./ci/scripts/doc_prettier_check.sh --write) to format files, commit the changes, and re-run the check.

And we can use --write option to fix.

I think there are several other potential auto fixes we can do, they can all be wrapped inside the lint script with --write option like

# best-effort auto fixes for all available places
./dev/rust_lint.sh --write

This idea is tracked in #19227

# commit the change.
#
# Ignore subproject CHANGELOG.md because it is machine generated
npx prettier@2.7.1 --check \
'{datafusion,datafusion-cli,datafusion-examples,dev,docs}/**/*.md' \
'!datafusion/CHANGELOG.md' \
README.md \
CONTRIBUTING.md
status=$?

if [ $status -ne 0 ]; then
echo "Prettier check failed. To fix, rerun `prettier` with --write (change --check to --write in the script), commit the formatted files, and re-run the check." >&2
exit $status
fi
1 change: 1 addition & 0 deletions dev/rust_lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ ci/scripts/rust_toml_fmt.sh
ci/scripts/rust_docs.sh
ci/scripts/license_header.sh
ci/scripts/typos_check.sh
ci/scripts/doc_prettier_check.sh
2 changes: 1 addition & 1 deletion docs/source/user-guide/metrics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ DataFusion operators expose runtime metrics so you can understand where time is
| Metric | Description |
| --------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| elapsed_compute | CPU time the operator actively spends processing work. |
| output_rows | Total number of rows the operator produces. |
| output_rows | Total number of rows the operator produces. |

@2010YOUY01 2010YOUY01 Dec 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Injecting this prettier violation to test CI works as before.

It failed in https://github.com/apache/datafusion/actions/runs/20094525462/job/57649510711?pr=19254

| output_bytes | Memory usage of all output batches. Note: This value may be overestimated. If multiple output `RecordBatch` instances share underlying memory buffers, their sizes will be counted multiple times. |
| output_batches | Total number of output batches the operator produces. |

Expand Down
Loading