Skip to content

Commit 68559f1

Browse files
authored
ci: run pre-commit on pull requests (#18)
## What / why This repo ships a `.pre-commit-config.yaml` whose hooks were not consistently enforced. This PR makes the pre-commit hooks run reliably in CI and green. ## Changes ### 1. `ci: skip: [go-fmt, go-vet]` in `.pre-commit-config.yaml` This is the key fix. Go formatting/vetting is **already enforced by the golangci-lint workflow (`lint.yml`)**, and the `dnephin/pre-commit-golang` **`go-vet` hook is broken for this repo's layout** — it fails with `no Go files in <root>` because the packages live in subdirectories under a single root `go.mod`. Skipping both Go hooks on pre-commit.ci makes that status green; Go quality stays covered by golangci-lint. ### 2. A native `pre-commit` GitHub Actions workflow Runs `uvx pre-commit run --all-files --show-diff-on-failure` via `astral-sh/setup-uv` on PRs and pushes to `main` (pre-commit.ci only covers PRs). Read-only, SHA-pinned, caches `~/.cache/pre-commit`. It uses `SKIP: go-fmt,go-vet` for the same reason as above. (If you'd rather rely solely on pre-commit.ci, this workflow can be dropped — the `ci: skip` change above is the essential part.) ### 3. check-yaml multi-document support Added `args: [--allow-multiple-documents]` — the repo's k8s manifests (`config/manager/manager.yaml`, `deploy/install.yaml`, `examples/git-sync-sidecar/notebook.yaml`) are valid multi-document YAML that the default hook rejected. ### 4. Mechanical hook autofixes (previously unenforced) - **ruff (auto):** removed unused imports (`F401`) in `plugin/examples/with-cw.py`, `plugin/tests/test_status.py`; removed extraneous f-string prefixes (`F541`) in `plugin/kubectl_marimo/deploy.py`. - **ruff (manual, trivial):** dropped an unused `mock_echo` assignment (`F841`) in `plugin/tests/test_deploy.py` (kept the `mocker.patch(...)` call). - **whitespace/EOF fixers:** trailing blank line in `.devcontainer/devcontainer.json`, missing trailing newline in `hack/boilerplate.go.txt`. ### Verification (local, Go hooks skipped as scoped) ``` trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...............................................................Passed check for merge conflicts................................................Passed ruff.....................................................................Passed ruff-format..............................................................Passed go fmt..................................................................Skipped go vet..................................................................Skipped ``` Part of the marimo-team engineering-excellence initiative to make sure every repo's declared quality gates actually run — and pass — in CI.
1 parent 0d06c2b commit 68559f1

8 files changed

Lines changed: 45 additions & 8 deletions

File tree

.devcontainer/devcontainer.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,3 @@
2222

2323
"onCreateCommand": "bash .devcontainer/post-install.sh"
2424
}
25-

.github/workflows/pre-commit.yml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: pre-commit
2+
3+
on:
4+
pull_request:
5+
push:
6+
branches: [main]
7+
8+
permissions:
9+
contents: read
10+
11+
concurrency:
12+
group: ${{ github.workflow }}-${{ github.ref }}
13+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
14+
15+
jobs:
16+
pre-commit:
17+
runs-on: ubuntu-latest
18+
timeout-minutes: 15
19+
# The Go hooks (go-fmt, go-vet) are skipped here: Go formatting and vetting
20+
# are already enforced by the golangci-lint workflow (lint.yml), and the
21+
# dnephin/pre-commit-golang go-vet hook is incompatible with this repo's
22+
# module layout (fails with "no Go files in <root>"). This job scopes
23+
# pre-commit to the Python (ruff) and generic hooks that run reliably.
24+
env:
25+
SKIP: go-fmt,go-vet
26+
steps:
27+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
28+
- uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4
29+
- uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
30+
with:
31+
path: ~/.cache/pre-commit
32+
key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
33+
- run: uvx pre-commit run --all-files --show-diff-on-failure

.pre-commit-config.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1+
ci:
2+
# Go formatting/vetting is enforced by the golangci-lint workflow (lint.yml).
3+
# The dnephin/pre-commit-golang go-vet hook is also incompatible with this
4+
# repo's single-root-go.mod layout (fails with "no Go files in <root>"), so
5+
# skip both Go hooks on pre-commit.ci to keep it green.
6+
skip: [go-fmt, go-vet]
7+
18
repos:
29
- repo: https://github.com/pre-commit/pre-commit-hooks
310
rev: v5.0.0
411
hooks:
512
- id: trailing-whitespace
613
- id: end-of-file-fixer
714
- id: check-yaml
15+
args: [--allow-multiple-documents]
816
- id: check-merge-conflict
917

1018
- repo: https://github.com/astral-sh/ruff-pre-commit

hack/boilerplate.go.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ distributed under the License is distributed on an "AS IS" BASIS,
1212
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1313
See the License for the specific language governing permissions and
1414
limitations under the License.
15-
*/
15+
*/

plugin/examples/with-cw.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
@app.cell
1515
def check_mount():
1616
import os
17-
import subprocess
1817

1918
# Check default mount location
2019
mount_dir = "/home/marimo/notebooks/mounts"

plugin/kubectl_marimo/deploy.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ def ensure_cw_credentials(namespace: str | None) -> bool:
122122
return False
123123

124124
# Step 4: Show what we're about to do
125-
click.echo(f"\nS3 Credentials:")
125+
click.echo("\nS3 Credentials:")
126126
click.echo(f" Namespace: {ns_display}")
127127
click.echo(f" Secret: {secret_name} (will create)")
128128
click.echo(f" Source: ~/.s3cfg [{section}]")
129-
click.echo(f" Access Key: ***")
129+
click.echo(" Access Key: ***")
130130

131131
# Step 5: Confirm with user if in interactive terminal
132132
if sys.stdin.isatty():

plugin/tests/test_deploy.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ def test_uses_namespace_for_section_lookup(self, mocker, tmp_path):
482482
mocker.patch("os.path.expanduser", return_value=str(s3cfg))
483483
mocker.patch("os.path.exists", return_value=True)
484484
mocker.patch("sys.stdin.isatty", return_value=False)
485-
mock_echo = mocker.patch("kubectl_marimo.deploy.click.echo")
485+
mocker.patch("kubectl_marimo.deploy.click.echo")
486486

487487
mock_result = mocker.Mock()
488488
mock_result.returncode = 0

plugin/tests/test_status.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
"""Tests for status command."""
22

33
import json
4-
from pathlib import Path
54

6-
from click.testing import CliRunner
75

86
from kubectl_marimo.status import show_status, format_elapsed
97

0 commit comments

Comments
 (0)