Conversation
# Conflicts: # pnpm-lock.yaml
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes Bazel build configuration files and integrates the gapic-generator-typescript package directly into the pnpm workspace. It also updates TypeScript type assertions in generator.ts to resolve compilation issues and modifies the installation command in librarian.yaml. Feedback suggests removing the --force flag from the pnpm install command in librarian.yaml to prevent unnecessary slowdowns in the build pipeline.
…ignore-scripts to workflows
…files-pnpm-10 # Conflicts: # core/generator/gapic-generator-typescript/WORKSPACE # core/generator/gapic-generator-typescript/package.json # core/generator/gapic-generator-typescript/pnpm-lock.yaml
… root package.json
…rride to root package.json" This reverts commit 2cc19a6.
- generator-tests/update-api-list: --filter instead of full-workspace installs; sha256-verify protoc; deref symlinks in node_modules artifact - document why librarian.yaml needs --no-frozen-lockfile and how to undo it after the next generator tag - centralize the protobufjs v7/v8 casts behind one documented alias; drop dead npm-style overrides
This was referenced Sep 22, 2026
…10' into consolidate-pnpm-lockfiles-pnpm-10 # Conflicts: # .github/workflows/generator-tests.yaml # core/generator/gapic-generator-typescript/package.json # core/generator/gapic-generator-typescript/typescript/src/generator.ts
Contributor
Author
|
Closing. This PR has been split into two reviewable pieces, and its one remaining idea was dropped on purpose.
Dropped: moving the generator into the root pnpm workspace.
|
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.
Split into reviewable pieces. Land these first, then merge
mainhere and this PR shrinks to just the generator change:pnpm-lock.yamlfilesupdate-api-liston pnpmWhat's left here
The Bazel
WORKSPACEcontainednpm_translate_lock(pnpm_lock = "//:pnpm-lock.yaml"), hard-wiring Bazel to the generator's nested lockfile. So removing Bazel and moving the generator into the root workspace cannot be separated — they're one change.Hand-written surface is ~150 lines:
pnpm-workspace.yamltypescript/src/generator.tspackage.json@bazel/bazeliskand a dead npm-styleoverrides.github/workflows/generator-tests.yamlEverything else is generated-file deletion (
WORKSPACE,BUILD.bazel,MODULE.bazel,.bazelrc,package-lock.json, the nestedpnpm-lock.yaml) plus the root lockfile update. Reviewers: filter out*lock*and this is 4 files.On the protobufjs casts
proto3-json-serializer@4(latest) hard-depends onprotobufjs@^7; the generator wants^8. Hoisting into the root workspace loses the nestedpnpm.overridesthat used to unify them, so TypeScript now sees two protobufjs type trees.Fixing this with a root
pnpm.overridesentry does not work — it was tried in 2cc19a6 and broke all 18unitsshards, because it forces protobufjs@8 onto google-gax, which needs v7. The casts are the correct fix until upstream supports v8. Documented inline so nobody retries the override.Known follow-ups (not addressed here)
.bazeliskrc,repositories.bzl(loaded by the deletedWORKSPACE), andrules_typescript_gapic/survive unreferenced.generator-tests.yaml(lines ~159-180) are gated onsteps.generator-changes.outputs.changes, but no step definesid: generator-changes— they have never run. They also usegenerator/...paths missing thecore/prefix. These are the combined-library Speech/Tasks tests, and the only consumer ofrules_typescript_gapic/from (1).Needs a call on whether those tests should be revived or deleted.