✨ Add a client based on operationId#129
Conversation
Signed-off-by: Nathan Gendron <nato@coderabbit.ai>
📝 WalkthroughWalkthroughThe Bitbucket Cloud Swagger spec is refreshed (deprecated ChangesOpenAPI Spec Refresh, Factory Extraction, and Code-Gen Pipeline
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant openapi_sh as openapi.sh
participant curl as curl
participant openapi_ts as openapi-typescript
participant gen_clients as generate_clients.ts
participant lint as pnpm lint:fix
Dev->>openapi_sh: pnpm run openapi
openapi_sh->>curl: fetch Cloud Swagger JSON
openapi_sh->>openapi_ts: emit src/cloud/openapi/openapi-typescript.ts
openapi_sh->>curl: fetch Server Swagger JSON
openapi_sh->>openapi_ts: emit src/server/openapi/openapi-typescript.ts
openapi_sh->>gen_clients: node generate_clients.ts
gen_clients-->>openapi_sh: src/cloud/client.ts + src/server/client.ts
openapi_sh->>lint: pnpm run lint:fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)scripts/generate_clients.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.11][ERROR]: unable to find a config; path src/cloud/client.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.11][ERROR]: unable to find a config; path 🔧 markdownlint-cli2 (0.22.1)README.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/server/create_client.test.ts (1)
4-7: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider enhancing test coverage.
The current test only verifies that
createBitbucketServerClientreturns a defined object. Consider adding assertions to verify the returned client has the expectedGET,POST, etc. methods from theopenapi-fetchclient interface, or test that configuration options are properly passed through.💡 Example enhanced test
test("createBitbucketServerClient", ({ expect }) => { const client = createBitbucketServerClient() expect(client).toBeDefined() expect(client.GET).toBeDefined() expect(client.POST).toBeDefined() }) test("createBitbucketServerClient with options", ({ expect }) => { const client = createBitbucketServerClient({ baseUrl: "https://example.org/rest", }) expect(client).toBeDefined() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/create_client.test.ts` around lines 4 - 7, The test for createBitbucketServerClient only verifies that the client object is defined but does not validate the client's structure or functionality. Enhance the test by adding assertions that verify the returned client has the expected HTTP methods (GET, POST, etc.) from the openapi-fetch client interface. Additionally, add a separate test case that verifies configuration options like baseUrl are properly passed through when createBitbucketServerClient is called with options, ensuring both the client's API surface and its configuration handling are properly tested.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate_clients.ts`:
- Around line 112-113: The import statement for InitParam is using an internal
source path instead of the public API entry point. Change the import statement
for InitParam from "openapi-fetch/src/index.js" to use the main entry point
"openapi-fetch" directly. This ensures the code relies on the package's public
contract rather than internal implementation details that may change.
- Around line 70-76: The JSDoc template string in the jsdoc variable
construction can output literal "undefined" text when summary or description is
missing or undefined. Fix this by providing default values for summary and
description before they are interpolated into the template string. Use the
logical OR operator or optional chaining with nullish coalescing to ensure
summary defaults to an empty string or appropriate placeholder when undefined,
and similarly ensure the description mapping logic only executes when
description is a valid string, preventing undefined or empty strings from being
included in the generated JSDoc comment output.
In `@scripts/openapi.sh`:
- Around line 1-15: The shell script lacks strict error handling, which allows
it to continue executing even after partial failures from curl HTTP errors or
command failures, potentially producing invalid generated clients. Add `set -e`
at the beginning of the script to make it exit immediately on any command
failure, and add the `--fail` flag to all three curl commands (the ones fetching
swagger.v3.json for Bitbucket Cloud and Bitbucket Data Center) to ensure curl
exits with an error status on HTTP failures rather than silently continuing with
empty or partial responses.
---
Nitpick comments:
In `@src/server/create_client.test.ts`:
- Around line 4-7: The test for createBitbucketServerClient only verifies that
the client object is defined but does not validate the client's structure or
functionality. Enhance the test by adding assertions that verify the returned
client has the expected HTTP methods (GET, POST, etc.) from the openapi-fetch
client interface. Additionally, add a separate test case that verifies
configuration options like baseUrl are properly passed through when
createBitbucketServerClient is called with options, ensuring both the client's
API surface and its configuration handling are properly tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 85ef456d-d660-4c53-805b-c32b5077a357
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
package.jsonscripts/generate_clients.tsscripts/openapi.shscripts/pnpm_version_patch.shsrc/cloud/client.test.tssrc/cloud/client.tssrc/cloud/create_client.test.tssrc/cloud/create_client.tssrc/cloud/index.tssrc/cloud/interfaces/paths.test.tssrc/cloud/openapi/openapi-typescript.tssrc/cloud/openapi/swagger.v3.jsonsrc/server/client.test.tssrc/server/client.tssrc/server/create_client.test.tssrc/server/create_client.tssrc/server/index.tstsconfig.eslint.json
💤 Files with no reviewable changes (1)
- scripts/pnpm_version_patch.sh
Signed-off-by: Nathan Gendron <nato@coderabbit.ai>
Signed-off-by: Nathan Gendron <nato@coderabbit.ai>
Signed-off-by: Nathan Gendron <nato@coderabbit.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
18-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify which Cloud entry point this example is showing.
The prose says class-based usage, but this snippet still demonstrates
createBitbucketCloudClient. Either switch the example toBitbucketCloudClientor explicitly label this as the factory form; mixing both patterns here will confuse readers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 18 - 39, The prose introduces usage of the BitbucketCloudClient class but the code example demonstrates the createBitbucketCloudClient factory function instead. Either update the introductory text to clarify that this example shows the factory function approach, or replace the code example to demonstrate actual BitbucketCloudClient class instantiation and usage instead of the factory function createBitbucketCloudClient.
62-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSeparate the Server factory and class examples.
This section also blends the factory helper and the new class API: the first snippet instantiates
createBitbucketServerClient, while the surrounding text frames the class as the documented path. Please make the section consistently describe one entry point, or clearly split the two.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 62 - 92, The README.md Server section shows two different ways to instantiate a Bitbucket client (the createBitbucketServerClient factory function and the BitbucketServerClient class constructor), but the introductory text only frames the class as the documented approach, creating inconsistency. Either remove the factory function example and keep only the BitbucketServerClient class constructor example, or clearly add section headers and introductory text that explicitly explains both approaches as alternative ways to create the client, making it obvious to readers that both are valid options.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@README.md`:
- Around line 18-39: The prose introduces usage of the BitbucketCloudClient
class but the code example demonstrates the createBitbucketCloudClient factory
function instead. Either update the introductory text to clarify that this
example shows the factory function approach, or replace the code example to
demonstrate actual BitbucketCloudClient class instantiation and usage instead of
the factory function createBitbucketCloudClient.
- Around line 62-92: The README.md Server section shows two different ways to
instantiate a Bitbucket client (the createBitbucketServerClient factory function
and the BitbucketServerClient class constructor), but the introductory text only
frames the class as the documented approach, creating inconsistency. Either
remove the factory function example and keep only the BitbucketServerClient
class constructor example, or clearly add section headers and introductory text
that explicitly explains both approaches as alternative ways to create the
client, making it obvious to readers that both are valid options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: bb4e1457-3990-4c37-a37d-1b722f407683
📒 Files selected for processing (4)
README.mdscripts/generate_clients.tsscripts/openapi.shsrc/cloud/client.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/openapi.sh
- scripts/generate_clients.ts
Summary by CodeRabbit
Release Notes
Bug Fixes
DRAFTandQUEUEDare no longer supported.New Features
Documentation
pnpm add.Chores