rusackas commented on code in PR #44738:
URL: https://github.com/apache/superset/pull/44738#discussion_r4141694795
##########
.github/workflows/pre-commit.yml:
##########
@@ -137,20 +117,90 @@ jobs:
if [ "$(printf '%s' "${files}" | wc -c)" -gt 100000 ]; then
echo "::notice::Changed-file list too large to pass via env;
falling back to --all-files."
echo "mode=all" >> "$GITHUB_OUTPUT"
+ emit_needs all
exit 0
fi
if [ -z "${files}" ]; then
echo "mode=none" >> "$GITHUB_OUTPUT"
+ emit_needs none
else
echo "mode=files" >> "$GITHUB_OUTPUT"
{
echo "files<<__CHANGED_FILES_EOF__"
echo "${files}"
echo "__CHANGED_FILES_EOF__"
} >> "$GITHUB_OUTPUT"
+ # scripts/pre_commit_dependency_needs.py (tests:
+ # tests/unit_tests/scripts/pre_commit_dependency_needs_test.py)
+ # writes needs_frontend/needs_docs straight to $GITHUB_OUTPUT. A
+ # prior inline `printf | grep -q` version risked a false "not
+ # needed" result: under `pipefail`, grep -q's early exit on a
+ # match can SIGPIPE a producer still mid-write for a large
+ # enough list (our own 100000-byte cap above is bigger than a
+ # typical 64KB pipe buffer), silently skipping a needed install
+ # instead of running it. Plain Python reading stdin to EOF has
+ # no equivalent early-exit-close behavior.
+ printf '%s\n' "${files}" | python3
scripts/pre_commit_dependency_needs.py
fi
+ - name: Setup Python
+ uses: $/.github/actions/setup-backend/
+ with:
+ python-version: ${{ matrix.python-version }}
+
+ - name: Setup Go
+ uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e #
v7.0.0
+
+ - name: Install helm-docs
+ run: go install github.com/norwoodj/helm-docs/cmd/[email protected]
+
+ # Spike: run the existing .pre-commit-config.yaml through prek (a Rust
+ # reimplementation of pre-commit) instead of pre-commit itself, to see
+ # whether it's viable to speed up this job. CI-only — contributors keep
+ # installing/running `pre-commit` locally exactly as documented; nothing
+ # here changes that.
+ - name: Install prek
+ run: |
+ curl --proto '=https' --tlsv1.2 -LsSf
https://github.com/j178/prek/releases/download/v0.4.11/prek-installer.sh | sh
+
+ - name: Setup Node.js
+ if: steps.changed_files.outputs.needs_frontend == 'true' ||
steps.changed_files.outputs.needs_docs == 'true'
Review Comment:
Good catch, added a `needs_websocket` output (mirrors oxfmt-websocket's own
path pattern) that gates just the Node setup step, not the frontend/docs
installs (102cdf59).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
