1.23.0 release - #929
Merged
Merged
Conversation
…#923) ## Problem `tests/clickhouse-test-runner` pinned `@clickhouse/rowbinary` to `^0.1.2`, pulling the **published** parser from npm rather than the in-repo `0.2.0`. CI and local runs therefore exercised the old decoder behavior (notably enum decoding), invalidating the new RowBinary backend coverage. Building `0.2.0` surfaced a second issue the pin was masking: `0.2.0` reorganized `dist/` into `readers/` and `writers/` subdirs (the recent "split source by direction" change), so the test-runner's old flat subpath imports (`@clickhouse/rowbinary/decimals`, `/core`, …) **no longer resolve** under the new export map. ## Changes - **Root `package.json`** — add `./skills/clickhouse-js-node-rowbinary` to `workspaces` so npm symlinks the local package, exactly like `@clickhouse/client` and `@clickhouse/datatype-parser`. The skill's `node_modules` is gitignored and its standalone build is driven by its own committed lockfile, so publishing the skill is unaffected. - **test-runner `package.json`** — `@clickhouse/rowbinary: "^0.1.2"` → `"*"` (matches the existing `@clickhouse/client` workspace reference). - **7 import sites** (3 files) — flat subpaths → `readers/*`. - `package-lock.json` regenerated: `node_modules/@clickhouse/rowbinary` is now a workspace link; no `0.1.2` registry refs remain. ## Verification - `npm install` links the local package ✓ - `datatype-parser`, `rowbinary`, `client` build ✓ - test-runner `typecheck` ✓, `build` ✓, `lint` ✓, **80/80 unit tests pass** against the linked `0.2.0` ✓ ## Note `"*"` is correct while rowbinary is developed in-repo. Once it's published at `0.2.0`+, consider re-pinning to a real semver range. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Summary Removes the `head` publish job from the deprecated `@clickhouse/client-common` publish workflow (`.github/workflows/publish-client-common.yml`). `@clickhouse/client-common` is deprecated — the `client` and `client-web` packages now bundle the common sources directly from `packages/client-common/src`. There's no longer a need to cut per-commit `-head.*` pre-releases from the release branch for this package. The `publish` job (the manual `workflow_dispatch` release) is left intact. ## Test plan - [ ] CI passes (workflow YAML lint) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Syncs the release branch with main to prepare the @clickhouse/client 1.23.0 release, including CI/workflow adjustments and test-runner updates so the in-repo @clickhouse/rowbinary implementation is exercised during tests instead of the published npm version.
Changes:
- Removes the
headpre-release publish job from the deprecated@clickhouse/client-commonworkflow. - Updates the ClickHouse test runner to consume the in-repo
@clickhouse/rowbinaryskill via a local link, adjusts imports, and tweaks TSV enum rendering expectations. - Updates lockfiles and pins
@types/nodein the RowBinary skill (and its lockfile) to align with the repo.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/clickhouse-test-runner/src/tsv-serialize.ts | Updates RowBinary helper imports and changes enum TSV rendering to treat enum values as already-resolved names. |
| tests/clickhouse-test-runner/src/backends/rowbinary.ts | Updates RowBinary backend imports for the dynamic RowBinaryWithNamesAndTypes decoder. |
| tests/clickhouse-test-runner/package.json | Switches @clickhouse/rowbinary dependency to a local file: link and adds a build step for the skill. |
| tests/clickhouse-test-runner/tests/tsv-serialize.test.ts | Updates RowBinary import and adjusts enum-related assertions to match new decoded shapes. |
| skills/clickhouse-js-node-rowbinary/package.json | Pins @types/node version used by the skill. |
| skills/clickhouse-js-node-rowbinary/package-lock.json | Lockfile updates reflecting the @types/node pin and its dependency changes. |
| package-lock.json | Root lockfile updates to link @clickhouse/rowbinary to the in-repo skill and reflect dependency changes. |
| .github/workflows/publish-client-common.yml | Removes the automatic head publish job for deprecated @clickhouse/client-common. |
Files not reviewed (1)
- skills/clickhouse-js-node-rowbinary/package-lock.json: Generated file
Comment on lines
+37
to
+40
| import { formatDecimal } from "@clickhouse/rowbinary/readers/decimals"; | ||
| import { formatTime, formatTime64 } from "@clickhouse/rowbinary/readers/time"; | ||
| import { formatUUID } from "@clickhouse/rowbinary/readers/uuid"; | ||
| import { formatIPv4, formatIPv6 } from "@clickhouse/rowbinary/readers/ip"; |
Comment on lines
+1
to
+2
| import { compileRowBinaryWithNamesAndTypes } from "@clickhouse/rowbinary/readers/rowBinaryWithNamesAndTypes"; | ||
| import { Cursor } from "@clickhouse/rowbinary/readers/core"; |
| import { describe, expect, it } from "vitest"; | ||
| import { parseDataType } from "@clickhouse/datatype-parser"; | ||
| import { formatUUID } from "@clickhouse/rowbinary/uuid"; | ||
| import { formatUUID } from "@clickhouse/rowbinary/readers/uuid"; |
Comment on lines
+15
to
18
| "build": "npm run build:rowbinary && rm -rf dist && tsc -p tsconfig.build.json && chmod +x dist/main.js", | ||
| "//build:rowbinary": "The in-repo @clickhouse/rowbinary skill is linked via file: (not a workspace, so it stays out of the root install/lint/test sweeps and keeps its own standalone CI). Its dist is gitignored, so build it here — the test runner is the last workspace built, so packages/* (datatype-parser) are already compiled when the skill's tsc resolves them.", | ||
| "build:rowbinary": "tsc -p ../../skills/clickhouse-js-node-rowbinary/tsconfig.build.json", | ||
| "typecheck": "tsc --noEmit", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync
releasefrommainto cut@clickhouse/client1.23.0.Brings
releaseup tomain(currently 2 commits behind: #928, #923). Version is already bumped to 1.23.0 onmain;packages/client-node/CHANGELOG.mdhas the# 1.23.0section ready.Merging this triggers the path-scoped
headpublish + e2e for@clickhouse/client(the synced commits touchpackage-lock.jsonandskills/**). After head e2e is green,latestis published via manualpublish-client.ymldispatch.