-
Notifications
You must be signed in to change notification settings - Fork 79
refactor!: generated SDK from upstream API specs #437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repo | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up node | ||
| uses: actions/setup-node@v3 | ||
|
|
||
| - name: Compile | ||
| run: yarn && yarn build | ||
|
|
||
| test: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix the problem, add an explicit permissions block to the workflow file to specify the minimum required permissions for the GITHUB_TOKEN. Since none of the jobs shown adds/updates any repository data (issues, PRs, contents, etc.), only contents: read is needed. The best and simplest way is to add a permissions: block at the root of the workflow, so it applies to all jobs unless overridden.
Where to change:
Edit the start of .github/workflows/ci.yml, after the name and before or after the on: block (YAML allows both positions, but commonly put after name: and before on: for clarity).
What else is needed:
No methods, imports, or definitions are needed. Only the new YAML block is inserted.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: ci | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: [push] | ||
|
|
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repo | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up node | ||
| uses: actions/setup-node@v3 | ||
|
|
||
| - name: Compile | ||
| run: yarn && yarn test |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
The best way to address this issue is to add a permissions block either to the root level of the workflow file (so it applies to all jobs), or to each job individually. Since neither job requires write access — only source code checkout and typical compile/test steps are performed — the minimal required permission is contents: read. To fix, add a permissions block at the root of .github/workflows/ci.yml (e.g., after the name: or on: block, before jobs:). No changes are needed elsewhere. The fix is a single YAML block addition.
-
Copy modified lines R5-R7
| @@ -2,6 +2,9 @@ | ||
|
|
||
| on: [push] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| compile: | ||
| runs-on: ubuntu-latest |
|
|
||
| // Handle dynamic imports (yield import, await import, regular import()) | ||
| const dynamicRegex = new RegExp( | ||
| `(yield\\s+import|await\\s+import|import)\\s*\\(\\s*['"](\\.\\.\?\\/[^'"]+)(\\${oldExt})['"]\\s*\\)`, |
Check failure
Code scanning / CodeQL
Useless regular-expression character escape High
regular expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix this problem, replace the unnecessary escape sequence \? in the regular expression string (line 59) with the correct regex that matches either ./ or ../ prefixes. This means we want either one or two dots followed by a slash. The best regex is \.{1,2}/, which matches ./ or ../. Update the string literal in the RegExp constructor in both staticRegex and dynamicRegex patterns, if necessary. Only update this in the code region provided, and do not change logic elsewhere.
-
Copy modified line R54 -
Copy modified line R59
| @@ -51,12 +51,12 @@ | ||
| // Update each extension type defined in the map | ||
| for (const [oldExt, newExt] of Object.entries(extensionMap)) { | ||
| // Handle static imports/exports | ||
| const staticRegex = new RegExp(`(import|export)(.+from\\s+['"])(\\.\\.?\\/[^'"]+)(\\${oldExt})(['"])`, "g"); | ||
| const staticRegex = new RegExp(`(import|export)(.+from\\s+['"])(\\.{1,2}\\/[^'"]+)(\\${oldExt})(['"])`, "g"); | ||
| newContent = newContent.replace(staticRegex, `$1$2$3${newExt}$5`); | ||
|
|
||
| // Handle dynamic imports (yield import, await import, regular import()) | ||
| const dynamicRegex = new RegExp( | ||
| `(yield\\s+import|await\\s+import|import)\\s*\\(\\s*['"](\\.\\.\?\\/[^'"]+)(\\${oldExt})['"]\\s*\\)`, | ||
| `(yield\\s+import|await\\s+import|import)\\s*\\(\\s*['"](\\.{1,2}\\/[^'"]+)(\\${oldExt})['"]\\s*\\)`, | ||
| "g", | ||
| ); | ||
| newContent = newContent.replace(dynamicRegex, `$1("$2${newExt}")`); |
Proposed changes
First wave of generated changes pre-configuration using Fern. Work to do.
Types of changes
What types of changes does your code introduce to the community JavaScript SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments