1.23 beta - #876
Merged
Merged
Conversation
## Summary
QA workflows should validate every PR targeting the `release` branch.
All test/examples workflows already do (their path filters match the
version bump a release PR always carries under
`packages/**`/`package-lock.json`), but `e2e-skills` was the only QA
workflow with **no `pull_request` trigger at all** — it never ran on any
PR.
- **`e2e-skills.yml`**: add a path-free `pull_request` trigger scoped to
the `release` branch, so the skills packaging E2E runs unconditionally
on release-branch PRs.
```yaml
pull_request:
branches:
- release
```
A path-free `branches: [release]` trigger is used because GitHub ANDs
`branches` + `paths` within a single `pull_request` trigger, and a
workflow cannot declare two — this guarantees the suite runs regardless
of which files changed.
Out of scope (separate lifecycles, left unchanged): `upstream-sql-tests`
(nightly upstream-compat harness) and `tests-skill-rowbinary-parser`
(independently-versioned `@clickhouse/rowbinary` package, not published
by `publish.yml`).
## Checklist
- [ ] For significant changes, documentation in
https://github.com/ClickHouse/clickhouse-docs was updated with further
explanations or tutorials
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…nto per-package jobs (#845) ## Summary `@clickhouse/client-common` requires platform hacks to serve both Node.js and Web well, and no longer pulls its weight as a separately-published package. This bundles the shared sources directly into each client, marks common deprecated, and restructures the bump/publish workflows into explicit per-package jobs. ## Changes ### Bundle common into each client - `packages/client-node/src/common` and `packages/client-web/src/common` are git-tracked symlinks to `packages/client-common/src`; both clients import via relative paths (`./common/index`) and compile their own copy into `dist/common`. - Dropped the `@clickhouse/client-common` dependency from both client `package.json` files — the clients are now self-contained. ### Mark common deprecated - `@deprecated` package-level notice in `client-common/src/index.ts`, a DEPRECATED package description, a CHANGELOG migration note (under `1.22.0`), and an `npm deprecate` step in `RELEASING.md`. ### Per-package release workflows - `bump-version.yml` and `publish.yml` are each split into **one job per package**, selected by a required `package` input (`@clickhouse/client`, `@clickhouse/client-web`, or `@clickhouse/client-common`). There is no `all` option — release a package by dispatching its workflow; to release several, dispatch once per package. The automatic `head` publish (on push to `release`) still runs a job per package and releases them all together. - Version bumping is inlined into the jobs (`npm version` + the `src/version.ts` rewrite); the shared `.scripts/update_version.sh` and the short-lived `head_version.sh` helper were removed. `RELEASING.md` is rewritten to be workflow-driven. - Manual publishes are scoped by package in the concurrency group so dispatching one does not cancel another; each per-package publish job builds and publishes only its own workspace and pushes a package-scoped git tag via `.scripts/push_release_tag.sh`. The `e2e` job runs whenever `@clickhouse/client` is published (head or manual), since the downstream test project imports it. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> 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
This PR deprecates @clickhouse/client-common as a runtime dependency by bundling its sources directly into @clickhouse/client (Node.js) and @clickhouse/client-web (Web), updates internal imports accordingly, and adjusts release/publish automation to support per-package publishing with additional pre-publish smoke checks.
Changes:
- Switched Node/Web clients from
@clickhouse/client-commonimports to local./common/indexre-exports and removed the@clickhouse/client-commondependency from theirpackage.json. - Marked
@clickhouse/client-commonas deprecated (package docs + npm metadata) and documented the migration. - Reworked release automation (bump/publish workflows) and added a tarball-install smoke test to catch packaging/export issues before publishing.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/smoke/check.mjs | New ESM smoke test to validate bundled common-origin exports from a packed install. |
| tests/e2e/smoke/check.cjs | New CJS smoke test to validate bundled common-origin exports from a packed install. |
| RELEASING.md | Updated release process docs for per-package bump/publish and deprecation guidance. |
| packages/client-web/src/utils/encoder.ts | Switched common imports to bundled ../common/index. |
| packages/client-web/src/result_set.ts | Switched common imports to bundled ./common/index. |
| packages/client-web/src/index.ts | Updated public re-exports to source from bundled common and clarified comments. |
| packages/client-web/src/connection/web_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-web/src/config.ts | Switched common imports to bundled ./common/index. |
| packages/client-web/src/client.ts | Switched common imports to bundled ./common/index. |
| packages/client-web/package.json | Removed runtime dependency on @clickhouse/client-common. |
| packages/client-node/src/utils/encoder.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/result_set.ts | Switched common imports to bundled ./common/index. |
| packages/client-node/src/index.ts | Updated public re-exports to source from bundled common and clarified comments. |
| packages/client-node/src/connection/stream.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/socket_pool.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/node_https_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/node_http_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/node_custom_agent_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/node_base_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/create_connection.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/connection/compression.ts | Switched common imports to bundled ../common/index. |
| packages/client-node/src/config.ts | Switched common imports to bundled ./common/index. |
| packages/client-node/src/client.ts | Switched common imports to bundled ./common/index. |
| packages/client-node/package.json | Removed runtime dependency on @clickhouse/client-common. |
| packages/client-common/src/index.ts | Added package-level deprecation notice for @clickhouse/client-common. |
| packages/client-common/package.json | Updated npm description to clearly indicate deprecation and migration targets. |
| package-lock.json | Removed client-common dependency edges for node/web workspaces. |
| CHANGELOG.md | Added migration note about deprecating @clickhouse/client-common. |
| AGENTS.md | Documented the new bundling approach via src/common symlink and relative imports. |
| .scripts/update_version.sh | Removed old jq-based multi-package version bump script. |
| .scripts/smoke_test_pack.sh | Added tarball pack/install/run smoke test for pre-publish validation. |
| .scripts/push_release_tag.sh | Added helper script to push package-scoped annotated release tags. |
| .github/workflows/publish.yml | Refactored publish workflow to per-package jobs, added smoke test, and package-scoped tagging. |
| .github/workflows/e2e-skills.yml | Ensured skills E2E runs on PRs targeting release. |
| .github/workflows/bump-version.yml | Refactored bump workflow to bump a selected package and open a PR automatically. |
## Summary Addresses the Copilot review comments on #876. - **CHANGELOG.md** — moved the `@clickhouse/client-common` deprecation *Migration Notes* from the `1.22.0` heading to the unreleased `1.23.0` heading, since that observable change (node/web no longer depend on client-common) ships in this release. - **RELEASING.md** — clarified the npm `11.x` requirement: it's newer than the npm bundled with Node.js `20.x`/`22.x` (`10.x`); either upgrade npm in place or use Node.js `24.x` (which ships npm `11.x`). - **tests/e2e/smoke/check.mjs** / **check.cjs** — use `assert.strictEqual` instead of loose `assert.equal`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
A short description of the changes with a link to an open issue.
Checklist
Delete items not relevant to your PR: