Migrate handwritten bigquery storage#8655
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures and updates the @google-cloud/bigquery-storage package, including updating the README with comprehensive documentation and code samples, adjusting package dependencies, and correcting import paths in generated samples from @google-cloud/storage to @google-cloud/bigquery-storage. Feedback on these changes highlights several issues in package.json, such as an incorrect repository field, a misleading description and keywords, and a non-standard engines.node version prefix. Crucially, the reviewer noted that the auto-generated src/index.ts has overwritten handwritten exports (such as managedwriter and reader), which will break backwards compatibility and cause the README examples to fail.
| }, | ||
| "version": "0.1.0", | ||
| "description": "Storage client for Node.js", | ||
| "repository": "googleapis/nodejs-storage", |
There was a problem hiding this comment.
The repository field is set to "googleapis/nodejs-storage", which is the repository for @google-cloud/storage. Since this package is @google-cloud/bigquery-storage and resides in the google-cloud-node monorepo, the repository field should point to googleapis/google-cloud-node with the correct directory, or use the previous repository object.
"repository": {
"type": "git",
"directory": "handwritten/bigquery-storage",
"url": "https://github.com/googleapis/google-cloud-node.git"
},| export {v1, v1alpha, v1beta, v1beta1, v1beta2, BigQueryReadClient, BigQueryWriteClient}; | ||
| export default {v1, v1alpha, v1beta, v1beta1, v1beta2, BigQueryReadClient, BigQueryWriteClient}; |
There was a problem hiding this comment.
The auto-generated src/index.ts has overwritten the handwritten exports (managedwriter, WriterClient, reader, ReadClient, adapt). This will break backwards compatibility and cause the examples in README.md to fail. Since this file is auto-generated, do not manually edit it; instead, configure the generator/synthtool to preserve these exports or add src/index.ts to the keep list in librarian.yaml.
References
- Do not manually edit generated TypeScript client files. Any changes or updates must be made upstream in the generator.
| "url": "https://github.com/googleapis/google-cloud-node.git" | ||
| }, | ||
| "version": "0.1.0", | ||
| "description": "Storage client for Node.js", |
There was a problem hiding this comment.
The description is set to "Storage client for Node.js", which is misleading as this is the BigQuery Storage client, not the Cloud Storage client. It should be "BigQuery Storage client for Node.js" or "Client for the BigQuery Storage API".
| "description": "Storage client for Node.js", | |
| "description": "Client for the BigQuery Storage API", |
| "google storage", | ||
| "storage", |
There was a problem hiding this comment.
The keywords "google storage" and "storage" refer to Google Cloud Storage. They should be updated to "google bigquery storage" and "bigquery storage" to avoid confusion.
| "google storage", | |
| "storage", | |
| "google bigquery storage", | |
| "bigquery storage", |
| "node": ">=18" | ||
| }, | ||
| "homepage": "https://github.com/googleapis/google-cloud-node/tree/main/handwritten/bigquery-storage" | ||
| "node": ">=v18" |
There was a problem hiding this comment.
The v prefix in ">=v18" is non-standard for the engines.node field in package.json and can cause issues with some package managers or semver parsers. It should be ">=18".
| "node": ">=v18" | |
| "node": ">=18" |
1 participant
(This is clearly nowhere near ready for merging, and was created using a version of librarian with local code.)