docs(readme): lead the SEA/kernel section with the consumer path - #463
Conversation
The Building > SEA/kernel section led with the Rust + make kernel-lib source build and said the archive is 'not committed - build it first', which is misleading for consumers: the prebuilt per-platform libdatabricks_sql_kernel.a is pulled automatically as a Go module dependency, so a kernel build needs only a C toolchain and -tags databricks_kernel - no Rust, no build step. - Rewrite SEA/kernel build steps to lead with the consumer path; demote the source build to a 'Build from source (contributors)' subsection. - Fix the at-a-glance table (no Rust for consumers; prebuilt auto-linked; list the 7 supported platforms). - Correct the stale 'make test-kernel requires make kernel-lib' note (the target links the prebuilt bindings; no kernel-lib needed). - Tighten the intro note and Cloning section. Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Docs-only README refactor that leads the SEA/kernel section with the consumer path. Claims check out against go.mod (7 kernel-bindings modules match the listed platforms) and the Makefile (test-kernel has no kernel-lib prerequisite and links the bindings modules — the old "requires make kernel-lib" note was indeed stale). One low-severity nuance: the new "build from source" contributor sequence implies make build-kernel/test-kernel link the locally built archive, but the Makefile says they link the published bindings modules unless a still-unwired go.work step is set up.
|
|
||
| Stage a prebuilt archive without Rust via | ||
| `make kernel-lib KERNEL_LOCAL_A=/path/to/libdatabricks_sql_kernel.a`. | ||
|
|
There was a problem hiding this comment.
🔵 Low — The contributor sequence presents make kernel-lib → make build-kernel → make test-kernel as a chain that builds and then links the local source archive. But per the Makefile's own comments, build-kernel/test-kernel link the published bindings modules, not the archive that kernel-lib drops into $(KERNEL_LIB_DIR); wiring the local build in requires a manual go.work step that is still an unwired TODO(dev-loop). So a contributor following these three lines will build a local .a and then silently build/test against the released bindings instead of their local build — the opposite of what this "build from source" subsection implies. Consider noting that linking the freshly built archive currently requires manual go.work setup.
Summary
The README's Building → SEA/kernel section led with the Rust +
make kernel-libsource build and stated the archive is "not committed — build it first" — misleading for consumers. In the v1.15.0 distribution model, the prebuilt per-platformlibdatabricks_sql_kernel.ais pulled automatically as a Go module dependency, so a kernel build needs only a C toolchain and-tags databricks_kernel— no Rust, no build step.Changes (docs-only)
CGO_ENABLED=1 go build -tags databricks_kernel, prebuilt archive auto-linked, 7 supported platforms); demote the source build to a "Build from source (contributors)" subsection.Verified against v1.15.0: a clean-room
go get+CGO_ENABLED=1 go build -tags databricks_kernel(no proxy, no Rust) links the prebuilt bindings and runs real queries.This pull request was AI-assisted by Isaac.