Repository navigation
docs(ci): explain workflow generation and merge-queue lint - #711
Conversation
6c759e7 to
7df0298
Compare
7df0298 to
e091c8c
Compare
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a clean CI PR. Coverage workflow rename (Rust Test→Rust Coverage) is consistent end-to-end (artifact name rust-coverage matches upload/download), and merge_queue_lint.yaml's job name and steps mirror dev.yaml's lint & pre-commit job so the required check is reported for merge groups. One low/unverified concern about the .github/zizmor.yaml filename extension vs zizmor's auto-discovery, filed inline. Nit (summary-only): clippy now runs twice on the test jobs — post-test.sh runs cargo clippy --all-targets --all-features -- -D warnings and the subsequent generated clippy step runs cargo clippy --no-deps -A unused_variables -D warnings; this is redundant but appears intentional (the post-test invocation is the stricter one), so no change needed.
| @@ -0,0 +1,69 @@ | |||
| # Copyright (c) 2026 ADBC Drivers Contributors | |||
There was a problem hiding this comment.
🔵 Low — The exceptions file is named .github/zizmor.yaml (.yaml), but both zizmor invocations (dev.yaml and merge_queue_lint.yaml) run zizmorcore/zizmor-action with no explicit config: input, relying on auto-discovery. zizmor's documented auto-discovery looks for zizmor.yml / .github/zizmor.yml — I could not confirm from here that the .yaml spelling is also discovered. If it is not, this entire exceptions file is silently ignored: the # zizmor: ignore[...] inline comments still work, but every per-rule/per-file suppression here (adhoc-packages, artipacked, excessive-permissions, template-injection, etc.) would have no effect, so either the zizmor step fails on the listed custom workflows or the suppressions give false confidence. Please confirm zizmor v0.6.4 discovers .github/zizmor.yaml; if it only discovers .yml, rename the file (or pass an explicit --config .github/zizmor.yaml). Marked low/unverified since the PR states zizmor currently passes.
e091c8c to
60d8514
Compare
Mandukhai-Alimaa
changed the title
60d8514 to
80badf6
Compare
80badf6 to
87f5026
Compare
There was a problem hiding this comment.
Verdict: 1 Medium
Solid, well-scoped change — the merge-queue workflow correctly reproduces the lint & pre-commit check name on merge_group events (which dev.yaml doesn't fire on), so the required status check won't hang. One medium concern: the Python version is already out of sync with dev.yaml, undercutting the PR's own "keep in sync" contract.
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.x" | ||
|
|
There was a problem hiding this comment.
🟡 Medium — The setup-python step here pins python-version: "3.x", but the lint job in dev.yaml it is supposed to mirror pins python-version: "3.14" with the explicit comment "Stick to 3.14 as some hooks/dependencies are not ready for 3.15". This is a real divergence in the lint step:
"3.x"resolves to the latest available 3.y (e.g. 3.15+), the exact versiondev.yamldeliberately avoids. pre-commit hooks that fail under 3.15 would then fail in the merge queue even though the PR check (on 3.14) passed — blocking merges, or masking failures, depending on which side regresses.- It directly contradicts this PR's own stated contract. The README added in this PR says the
lint & pre-commitjob "matches the check fromdev.yaml" and "Keep their lint steps in sync." The Python version is part of that lint environment and is out of sync from day one.
Pin the same version as dev.yaml to keep the two checks behaviorally identical.
87f5026 to
94ffed7
Compare
Mandukhai-Alimaa
changed the title
There was a problem hiding this comment.
Verdict: 1 Medium
Docs-only PR; accurate on the parts I could verify (dev.yaml ↔ merge_queue_lint.yaml lint-job parity, and the zizmor.yaml exception categories all check out). One medium concern: the documented generate.toml config doesn't exist anywhere in this checkout, so the regeneration procedure isn't reproducible unless it lands via another PR in the stack.
| uvx --from git+https://github.com/adbc-drivers/dev adbc-gen-workflow generate "$(pwd)" | ||
| ``` | ||
|
|
||
| `generate.toml` enables only Go. Regeneration leaves the existing Rust workflows, |
There was a problem hiding this comment.
🟡 Medium — The doc describes a regeneration procedure that relies on generate.toml ("generate.toml enables only Go. Regeneration leaves the existing Rust workflows... unchanged."), but no generate.toml exists anywhere in this checkout — the only occurrence of that name in the repo is this README line. A developer following the documented command (uvx --from ... adbc-gen-workflow generate "$(pwd)") would not find the config the doc says controls generation, so the procedure as written isn't reproducible against the current tree.
If generate.toml is introduced by another PR in this stack that hasn't landed in this branch base (the merge-queue workflow and .github/zizmor.yaml from #709 are present, but this file is not), please confirm the stack ordering so the doc and the config land together. Otherwise the file should be added alongside this doc, or the reference path corrected.
94ffed7 to
d8503ab
Compare
…ers#708) Update driverbase/ADBC, migrate the Go driver to context-aware APIs, and regenerate `go/pkg` bindings. Pin lint to Python 3.14 so this PR can pass merge-queue checks independently. **Breaking for Go callers:** affected methods now require a context. Base of the stack: adbc-drivers#708 → adbc-drivers#709 → adbc-drivers#711 → adbc-drivers#713 → adbc-drivers#716 → adbc-drivers#718.
Update the Go validation suite, including shared dictionary-ingestion coverage, and adapt its pytest hooks, timestamp fixtures, and documentation generation. Support `--vendor-version latest`. Stacked on adbc-drivers#711; dictionary binding and ingestion support follows in adbc-drivers#718. --------- Signed-off-by: Mandy Alimaa <mandukhaialimaa@gmail.com>
Document Go-only workflow generation, merge-queue lint maintenance, and existing zizmor exceptions.
Stacked on #709, which includes the merge-queue workflow.