Add SDK build validation workflow for real spec files #1206
Conversation
|
Warning Rate limit exceeded@ChiragAgg5k has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a GitHub Actions workflow (.github/workflows/sdk-build-validation.yml) that generates and builds multiple SDKs via a matrix across client, server, and console SDKs with language-specific setup and per-SDK build steps. Updates example.php by adding a public configureSDK($sdk, $overrides = []) helper, supporting optional SDK and platform CLI arguments and conditional per-language generation when an SDK is requested. Modifies Swift/Apple package templates to conditionally add the Enums target as a dependency of the Models target when enums exist. Changes realtime endpoints from ws:// to wss:// and calls client.setSelfSigned(false) in Apple tests; web tests updated to use wss://. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces automated SDK generation and build validation using real Appwrite spec files instead of generic test specs. The implementation adds comprehensive CI/CD validation for all supported Appwrite SDKs across client, server, and console platforms.
- Adds a new GitHub Actions workflow for validating SDK builds using real specifications
- Creates a PHP script to generate individual SDKs with proper platform-specific configurations
- Implements matrix-based parallel execution for 14 different SDK types with their respective build toolchains
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| generate-sdk.php | PHP script that generates SDKs for specific platforms using real Appwrite spec files from GitHub |
| .github/workflows/sdk-build-validation.yml | GitHub Actions workflow with matrix strategy to build and validate all supported SDKs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
generate-sdk.php (4)
46-47: Parameterize the hardcoded version.The version '1.8.x' is hardcoded, making it difficult to test different versions or update when new versions are released.
Consider accepting version as an optional third argument with a default:
+$version = $argv[3] ?? '1.8.x'; -$version = '1.8.x';Update the usage message accordingly:
- echo "Usage: php generate-sdk.php <sdk-name> <platform>\n"; - echo "Example: php generate-sdk.php php server\n"; + echo "Usage: php generate-sdk.php <sdk-name> <platform> [version]\n"; + echo "Example: php generate-sdk.php php server 1.8.x\n"; echo "Platforms: client, server, console\n";
55-472: Significant code duplication across SDK cases.The switch statement contains substantial duplication—each case follows nearly identical patterns with only language-specific differences (package names, namespaces). This makes maintenance difficult and error-prone.
Consider extracting common configuration into a data structure:
$sdkConfigs = [ 'php' => [ 'class' => PHP::class, 'platform' => 'server', 'setup' => function($lang) { $lang->setComposerVendor('appwrite') ->setComposerPackage('appwrite'); } ], 'web' => [ 'class' => Web::class, 'platform' => 'client', 'version' => '0.0.0', ], // ... other configs ]; if (!isset($sdkConfigs[$sdkName])) { throw new Exception("Unknown SDK: $sdkName"); } $config = $sdkConfigs[$sdkName]; $language = new $config['class'](); if (isset($config['setup'])) { $config['setup']($language); } $sdk = new SDK($language, new Swagger2($spec)); // Apply common configuration $sdk->setName('NAME') ->setDescription('Repo description goes here') // ... etc ->generate(__DIR__ . "/examples/$sdkName");
175-180: Document why assistant and avatars services are excluded for CLI.The CLI SDK explicitly excludes 'assistant' and 'avatars' services without explanation. This should be documented to help future maintainers understand the rationale.
Add a comment explaining the exclusion:
+ // Exclude assistant and avatars services from CLI SDK + // as they are not supported in CLI context ->setExclude([ 'services' => [
476-483: Consolidate duplicate catch blocks.Both catch blocks have identical logic. Since
ExceptionextendsThrowable, a single catch forThrowablewill handle both.Apply this diff:
-catch (Exception $exception) { - echo 'Error: ' . $exception->getMessage() . ' on ' . $exception->getFile() . ':' . $exception->getLine() . "\n"; - exit(1); -} catch (Throwable $exception) { echo 'Error: ' . $exception->getMessage() . ' on ' . $exception->getFile() . ':' . $exception->getLine() . "\n"; exit(1); }.github/workflows/sdk-build-validation.yml (3)
7-7: Consider adding path filters to the pull_request trigger.The workflow runs on all pull requests. Since it validates SDK generation, consider limiting it to changes that affect SDK generation (e.g., changes to
generate-sdk.php,src/, or the workflow itself).-on: [pull_request] +on: + pull_request: + paths: + - 'generate-sdk.php' + - 'src/**' + - '.github/workflows/sdk-build-validation.yml' + - 'composer.json' + - 'composer.lock'
146-147: Cache Composer dependencies to speed up workflow.The workflow installs Composer dependencies on every run without caching, which slows down execution.
Add caching before the install step:
+- name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: vendor + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: | + ${{ runner.os }}-composer- + - name: Install Composer Dependencies run: composer install
153-207: Add dependency caching for language-specific package managers.The workflow sets up multiple language runtimes but doesn't cache their dependencies, leading to slower builds.
Add caching steps for each language. For example, after Node.js setup:
- name: Setup Node.js if: matrix.sdk == 'web' || matrix.sdk == 'node' || matrix.sdk == 'cli' || matrix.sdk == 'react-native' uses: actions/setup-node@v4 with: node-version: '20' cache: 'npm' cache-dependency-path: examples/${{ matrix.sdk }}/package-lock.jsonSimilar caching can be added for:
- Flutter: uses built-in caching with
flutter-action- Go: add
cache: truetosetup-go- .NET: add
cache: truetosetup-dotnet- Ruby: add
bundler-cache: truetosetup-ruby- Python: add
cache: 'pip'tosetup-python
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/sdk-build-validation.yml(1 hunks)generate-sdk.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
generate-sdk.php (9)
src/Spec/Spec.php (1)
Spec(8-193)src/SDK/SDK.php (6)
SDK(21-938)setName(257-262)setDefaultHeaders(235-240)generate(618-737)setExclude(750-759)setNamespace(246-251)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/Language/Dart.php (2)
Dart(8-508)setPackageName(21-26)src/SDK/Language/Flutter.php (1)
Flutter(5-360)src/SDK/Language/Android.php (1)
Android(5-349)src/SDK/Language/ReactNative.php (1)
ReactNative(7-256)src/SDK/Language/JS.php (1)
setNPMPackage(19-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: generate-and-build (dart, server, Dart, cd examples/dart
dart pub get
dart analyze
) - GitHub Check: generate-and-build (dotnet, server, DotNet, cd examples/dotnet
dotnet build
) - GitHub Check: generate-and-build (swift, server, Swift, cd examples/swift
swift build
) - GitHub Check: generate-and-build (apple, client, Apple, cd examples/apple
swift build
) - GitHub Check: generate-and-build (web, client, Web, cd examples/web
npm install
npm run build
) - GitHub Check: generate-and-build (flutter, client, Flutter, cd examples/flutter
flutter pub get
flutter analyze
) - GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, Go118)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, Android5Java17)
🔇 Additional comments (3)
.github/workflows/sdk-build-validation.yml (3)
149-150: LGTM! SDK generation step correctly uses matrix variables.The generation step properly invokes
generate-sdk.phpwith the matrix-defined SDK name and platform.
208-209: Build step executes matrix-defined commands correctly.The final build step properly uses the matrix-defined
build_cmdto validate each SDK.
154-154: Node.js setup condition covers all required SDKs (web, node, cli, react-native).
ChiragAgg5k
force-pushed
the
feat-sdk-build-validation-workflow
branch
from
35ebb3e to
36160be
Compare
This commit introduces a GitHub Actions workflow that generates and validates SDKs using the real Appwrite spec files instead of the generic test spec. - Add sdk-build-validation.yml workflow that generates SDKs from real spec files - Add generate-sdk.php script to generate individual SDKs by platform - SDKs are properly categorized by platform (client, server, console) - CLI SDK uses console platform as intended - Each SDK is built with its respective toolchain to validate successful compilation - Workflow runs on pull requests with matrix strategy for parallel execution Supported SDKs: - Client: web, flutter, apple, android, react-native - Server: node, php, python, ruby, dart, go, swift, dotnet, kotlin - Console: cli 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
ChiragAgg5k
force-pushed
the
feat-sdk-build-validation-workflow
branch
from
36160be to
3da9477
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/sdk-build-validation.yml (1)
139-192: Build commands suppress errors that should fail the workflow.Several build commands use
|| trueor|| echoto suppress failures:
- Line 153:
npm run build || echo "No build script..."- continues if build fails- Line 163:
chmod +x ./gradlew || true- silently continues if chmod fails- Line 181:
go mod tidy || true- continues if dependency resolution failsThese patterns prevent the workflow from catching actual build failures.
For React Native, check if build script exists:
npm install -npm run build || echo "No build script, checking syntax only" +if grep -q '"build"' package.json; then + npm run build +else + echo "No build script found, skipping" +fiFor Android/Kotlin, verify the file exists first:
-chmod +x ./gradlew || true +if [ -f ./gradlew ]; then chmod +x ./gradlew; fi ./gradlew buildFor Go, let mod tidy failures surface:
-go mod tidy || true +go mod tidy go build ./...
🧹 Nitpick comments (1)
example.php (1)
39-62: Add validation for the requested SDK parameter.The script accepts any value for
$requestedSdkwithout validation. If an invalid SDK name is provided, the script will silently skip all generation blocks and report success.Consider adding validation after parsing arguments:
// Parse command-line arguments $requestedSdk = isset($argv[1]) ? $argv[1] : null; $requestedPlatform = isset($argv[2]) ? $argv[2] : null; + +// Validate SDK name if provided +$validSdks = ['php', 'web', 'node', 'cli', 'ruby', 'python', 'dart', + 'flutter', 'react-native', 'go', 'swift', 'apple', + 'dotnet', 'rest', 'android', 'kotlin', 'graphql']; +if ($requestedSdk && !in_array($requestedSdk, $validSdks)) { + throw new Exception("Invalid SDK: $requestedSdk. Valid SDKs: " . implode(', ', $validSdks)); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/sdk-build-validation.yml(1 hunks)example.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (7)
src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/SDK.php (20)
SDK(21-938)setName(257-262)setDescription(268-273)setShortDescription(279-284)setURL(389-394)setLogo(378-383)setLicenseContent(323-328)setWarning(444-449)setChangelog(477-482)setGitUserName(356-361)setGitRepoName(345-350)setTwitter(512-517)setDiscord(500-506)setDefaultHeaders(235-240)generate(618-737)setVersion(290-295)setReadme(466-471)setLicense(312-317)setExamples(488-493)setNamespace(246-251)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)src/SDK/Language/JS.php (1)
setNPMPackage(19-24)src/SDK/Language/Dart.php (2)
Dart(8-508)setPackageName(21-26)src/SDK/Language/Flutter.php (1)
Flutter(5-360)src/SDK/Language/ReactNative.php (1)
ReactNative(7-256)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: web (client)
- GitHub Check: android (client)
🔇 Additional comments (6)
.github/workflows/sdk-build-validation.yml (5)
1-8: LGTM!The workflow metadata, concurrency control, and trigger configuration are appropriate for SDK validation on pull requests.
9-64: LGTM!The matrix strategy is well-structured with comprehensive SDK coverage across client, server, and console platforms. The
fail-fast: falsesetting ensures all SDK builds are validated even if some fail.
65-82: LGTM!The checkout, PHP setup, and SDK generation steps are correctly configured. The recursive submodule checkout and PHP 8.3 with curl extension align with the requirements of
example.php.
83-138: LGTM!All language-specific setup steps are correctly conditioned on the matrix SDK and use appropriate recent stable versions.
147-150: CLI build targets only Linux x64.The CLI build step runs
npm run linux-x64, which validates only the Linux x64 build. This might miss platform-specific issues in other target platforms (Windows, macOS, ARM).Consider whether this is sufficient for validation. If cross-platform compatibility is important, you may want to verify if the npm scripts support building multiple targets in CI, or if this limitation is acceptable given that the workflow runs on ubuntu-latest.
example.php (1)
65-526: LGTM!All SDK generation blocks are correctly wrapped with conditional logic that enables selective generation based on the
$requestedSdkparameter while maintaining backward compatibility when no SDK is specified.
Add AppwriteEnums as a dependency to the Models target in both Swift and Apple Package.swift templates to resolve "no such module 'AppwriteEnums'" compilation errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/sdk-build-validation.yml (3)
167-167: Build commands suppress errors that should fail the workflow.The
chmod +x ./gradlew || truepattern silently continues if chmod fails, preventing the workflow from catching actual build failures. The same issue exists at line 185 withgo mod tidy || true.This concern was already raised in a previous review. Please address the error suppression patterns at lines 167 and 185 as suggested in the earlier review.
157-157: Build command suppresses errors that should fail the workflow.The
npm run build || echo "No build script..."pattern continues if the build fails, preventing the workflow from catching actual build failures.This concern was already raised in a previous review. Please check if a build script exists in package.json and only skip with a clear message if absent, otherwise allow build failures to surface.
185-185: Build command suppresses errors that should fail the workflow.The
go mod tidy || truepattern continues if dependency resolution fails, preventing the workflow from catching actual build failures.This concern was already raised in a previous review. Please remove
|| trueso dependency errors fail the workflow as expected.
🧹 Nitpick comments (1)
.github/workflows/sdk-build-validation.yml (1)
96-104: Use Swift 6.1 in workflow
In .github/workflows/sdk-build-validation.yml (lines 96–104), update the Swift download URL and directory from swift-5.9.2-RELEASE to swift-6.1-RELEASE for the Ubuntu 22.04 installer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/sdk-build-validation.yml(1 hunks)templates/apple/Package.swift.twig(1 hunks)templates/swift/Package.swift.twig(1 hunks)tests/languages/apple/Tests.swift(1 hunks)tests/languages/web/index.html(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Go118)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: dotnet (server)
- GitHub Check: kotlin (server)
- GitHub Check: swift (server)
- GitHub Check: android (client)
- GitHub Check: react-native (client)
- GitHub Check: apple (client)
🔇 Additional comments (8)
tests/languages/web/index.html (1)
38-38: LGTM! Security improvement by using secure WebSocket.The change from
ws://towss://ensures encrypted communication for the Realtime endpoint. This aligns with the similar change in tests/languages/apple/Tests.swift.tests/languages/apple/Tests.swift (1)
35-35: LGTM! Consistent security improvement.The change from
ws://towss://for the Realtime endpoint ensures encrypted communication, matching the corresponding change in tests/languages/web/index.html.templates/apple/Package.swift.twig (1)
47-49: LGTM! Proper conditional dependency for enum usage.The conditional dependency ensures that the Models target can reference enum types when
spec.allEnumsis not empty. This mirrors the existing pattern for the main SDK target (lines 37-39) and aligns with the identical change in templates/swift/Package.swift.twig.templates/swift/Package.swift.twig (1)
47-49: LGTM! Proper conditional dependency for enum usage.The conditional dependency ensures that the Models target can reference enum types when
spec.allEnumsis not empty. This mirrors the existing pattern for the main SDK target (lines 37-39) and aligns with the identical change in templates/apple/Package.swift.twig..github/workflows/sdk-build-validation.yml (4)
1-64: LGTM! Well-structured workflow with comprehensive SDK coverage.The workflow configuration is solid:
- Proper concurrency control to cancel outdated runs
- Matrix includes 14 SDKs across client, server, and console platforms
fail-fast: falseallows all SDKs to be validated even if one fails
66-82: LGTM! Proper setup for SDK generation.The setup steps are correct:
- Submodules are checked out recursively
- PHP 8.3 with curl extension is appropriate for the generator
- The generation command properly passes SDK and platform parameters to example.php
84-141: LGTM! Comprehensive language setup with appropriate versions.The language-specific setup steps properly handle all required runtimes:
- Modern versions chosen for each language (Node 20, Java 17, Python 3.11, Go 1.21, .NET 8.0.x, Ruby 3.1)
- Conditionals correctly match SDK requirements
- Swift manual installation is necessary for Ubuntu
146-195: Build commands are comprehensive and well-organized.The case statement properly handles build requirements for each SDK type with appropriate toolchain commands. The logic correctly maps each SDK to its build process.
Note: Excluding the error suppression issues flagged separately, the build step structure is sound.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/languages/apple/Tests.swift(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/languages/apple/Tests.swift (2)
tests/languages/node/test.js (4)
foo(24-24)client(19-22)bar(25-25)general(26-26)tests/languages/web/node.js (4)
foo(9-9)client(7-7)bar(10-10)general(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Python311)
- GitHub Check: build (8.3, Python310)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: swift (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (3)
tests/languages/apple/Tests.swift (3)
35-153: Add explanation to PR description for realtime functionality removal.The systematic commenting out of realtime functionality across this test file (endpoint configuration, subscription logic, and assertions) is a significant change that removes test coverage for an entire feature area. While the modifications appear coordinated with similar changes in other test files, the PR description contains empty placeholders and lacks justification.
Please update the PR description with:
- What does this PR do? - Explain why realtime tests are being disabled
- Test Plan - Document how realtime functionality was verified (or why it's acceptable to skip)
- Related PRs and Issues - Link to any tracking issues for re-enabling realtime tests
This documentation is critical for future maintainers who may wonder why realtime coverage is missing.
35-35: Clarify disabling the realtime test configurationThe realtime endpoint setup in
tests/languages/apple/Tests.swiftis commented out without explanation. Is this a temporary measure for the build validation workflow or a permanent removal? This could impact SDK realtime support—please clarify and add a TODO comment if it’s meant to be re-enabled later.
152-153: Confirm realtime tests disabled in Swift and PHP
The realtime subscription, callback and assertions intests/languages/apple/Tests.swift(lines 35, 41–48, 152–153) and theREALTIME_RESPONSESentry intests/AppleSwift56Test.phpare commented out. If this is temporary, add a link to a tracking issue; if permanent, delete the commented code to clean up.
ChiragAgg5k
force-pushed
the
feat-sdk-build-validation-workflow
branch
from
5092353 to
c490d83
Compare
ChiragAgg5k
force-pushed
the
feat-sdk-build-validation-workflow
branch
from
d1345dd to
fa19212
Compare
Extracted repetitive SDK configuration logic into a configureSDK() helper function to reduce code duplication across all SDK definitions. Common fields are now defined once with defaults, and each SDK only specifies unique overrides. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
example.php (1)
356-365: UseconfigureSDKfor consistency.The GraphQL SDK generation doesn't use the
configureSDKhelper, unlike all other SDK blocks. This creates inconsistency and means GraphQL misses standard configurations (version, warning, changelog, git info, Twitter, Discord, default headers).Apply this diff to align with other SDK blocks:
// GraphQL if (!$requestedSdk || $requestedSdk === 'graphql') { $sdk = new SDK(new GraphQL(), new Swagger2($spec)); - $sdk - ->setName('GraphQL') - ->setDescription('Appwrite is an open-source backend as a service server that abstract and simplify complex and repetitive development tasks behind a very simple to use REST API. Appwrite aims to help you develop your apps faster and in a more secure way. Use the Flutter SDK to integrate your app with the Appwrite server to easily start interacting with all of Appwrite backend APIs and tools. For full API documentation and tutorials go to https://appwrite.io/docs') - ->setLogo('https://appwrite.io/v1/images/console.png'); + configureSDK($sdk, [ + 'name' => 'GraphQL', + 'description' => 'Appwrite is an open-source backend as a service server that abstract and simplify complex and repetitive development tasks behind a very simple to use REST API. Appwrite aims to help you develop your apps faster and in a more secure way. Use the Flutter SDK to integrate your app with the Appwrite server to easily start interacting with all of Appwrite backend APIs and tools. For full API documentation and tutorials go to https://appwrite.io/docs', + 'logo' => 'https://appwrite.io/v1/images/console.png', + 'version' => '0.0.1', + ]); $sdk->generate(__DIR__ . '/examples/graphql'); }
🧹 Nitpick comments (1)
example.php (1)
94-106: Validate the platform parameter.The
$platformvariable is used directly in the URL (line 109) without validation. While the risk is low since it's only used in a GitHub HTTPS URL, it would be more defensive to validate that the platform is one of the expected values.Apply this diff to add validation:
// Determine platform if ($requestedPlatform) { $platform = $requestedPlatform; } else { // Leave the platform you want uncommented // $platform = 'client'; $platform = 'console'; // $platform = 'server'; } + +$allowedPlatforms = ['client', 'server', 'console']; +if (!in_array($platform, $allowedPlatforms, true)) { + throw new Exception("Invalid platform: $platform. Allowed values: " . implode(', ', $allowedPlatforms)); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (3)
src/SDK/SDK.php (21)
setName(257-262)setDescription(268-273)setShortDescription(279-284)setURL(389-394)setLogo(378-383)setLicenseContent(323-328)setWarning(444-449)setChangelog(477-482)setGitUserName(356-361)setGitRepoName(345-350)setTwitter(512-517)setDiscord(500-506)setDefaultHeaders(235-240)setVersion(290-295)setExamples(488-493)setReadme(466-471)setLicense(312-317)setNamespace(246-251)setExclude(750-759)SDK(21-938)generate(618-737)src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: go (server)
- GitHub Check: react-native (client)
- GitHub Check: swift (server)
- GitHub Check: flutter (client)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (1)
example.php (1)
39-92: LGTM! Helper function addresses previous feedback.The
configureSDKhelper successfully addresses the past review comment about duplicating SDK configuration logic. The implementation correctly uses method chaining and handles optional overrides.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (5)
src/SDK/SDK.php (20)
setName(257-262)setVersion(290-295)setDescription(268-273)setShortDescription(279-284)setURL(389-394)setLogo(378-383)setLicenseContent(323-328)setWarning(444-449)setChangelog(477-482)setGitUserName(356-361)setGitRepoName(345-350)setTwitter(512-517)setDiscord(500-506)setDefaultHeaders(235-240)setReadme(466-471)setExamples(488-493)setLicense(312-317)setNamespace(246-251)setExclude(750-759)SDK(21-938)src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)src/SDK/Language/JS.php (1)
setNPMPackage(19-24)src/SDK/Language/Dart.php (2)
Dart(8-508)setPackageName(21-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, DotNet90)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: flutter (client)
- GitHub Check: apple (client)
🔇 Additional comments (2)
example.php (2)
39-90: LGTM! Well-designed helper function.This helper successfully addresses the previous feedback about duplicated configuration logic. The implementation is clean with sensible defaults, supports overrides, and follows the fluent API pattern appropriately.
92-112: LGTM!The CLI argument handling is straightforward and correct. The runtime messaging aids debugging, and the platform defaulting logic is appropriate for this example script.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
example.php (1)
283-290: Refactor GraphQL to use configureSDK helper for consistency.The GraphQL SDK block still does not use the
configureSDKhelper, meaning it won't receive standard defaults like version, warning, changelog, git information, default headers, readme, etc. This creates an inconsistency across generated SDKs.Apply this diff to align with other SDK blocks:
if (!$requestedSdk || $requestedSdk === 'graphql') { $sdk = new SDK(new GraphQL(), new Swagger2($spec)); - $sdk - ->setName('GraphQL') - ->setDescription('Appwrite is an open-source backend as a service server that abstract and simplify complex and repetitive development tasks behind a very simple to use REST API. Appwrite aims to help you develop your apps faster and in a more secure way. Use the Flutter SDK to integrate your app with the Appwrite server to easily start interacting with all of Appwrite backend APIs and tools. For full API documentation and tutorials go to https://appwrite.io/docs') - ->setLogo('https://appwrite.io/v1/images/console.png'); + configureSDK($sdk, [ + 'name' => 'GraphQL', + 'description' => 'Appwrite is an open-source backend as a service server that abstract and simplify complex and repetitive development tasks behind a very simple to use REST API. Appwrite aims to help you develop your apps faster and in a more secure way. Use the Flutter SDK to integrate your app with the Appwrite server to easily start interacting with all of Appwrite backend APIs and tools. For full API documentation and tutorials go to https://appwrite.io/docs', + ]); $sdk->generate(__DIR__ . '/examples/graphql'); }
🧹 Nitpick comments (1)
example.php (1)
54-54: Header name casing inconsistency between default and Android/Kotlin overrides.The default configuration uses
'X-Appwrite-Response-Format'(mixed case) while Android and Kotlin overrides use'x-appwrite-response-format'(lowercase). HTTP headers are case-insensitive, so this won't cause functional issues, but the inconsistency may be unintentional.If intentional (e.g., for backwards compatibility), consider adding a comment. Otherwise, standardize on one casing style.
Also applies to: 260-260, 277-277
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (5)
src/SDK/SDK.php (18)
setName(257-262)setVersion(290-295)setDescription(268-273)setShortDescription(279-284)setURL(389-394)setLogo(378-383)setLicenseContent(323-328)setWarning(444-449)setChangelog(477-482)setGitUserName(356-361)setGitRepoName(345-350)setTwitter(512-517)setDiscord(500-506)setDefaultHeaders(235-240)setReadme(466-471)setExclude(750-759)SDK(21-938)generate(618-737)src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)src/SDK/Language/JS.php (1)
setNPMPackage(19-24)src/SDK/Language/Android.php (1)
Android(5-349)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: cli (console)
- GitHub Check: kotlin (server)
- GitHub Check: swift (server)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: android (client)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: apple (client)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, Android14Java17)
🔇 Additional comments (3)
example.php (3)
39-81: LGTM! The configureSDK helper successfully centralizes SDK configuration.This helper function addresses the previous feedback about duplicating SDK configuration logic. It provides a clean interface for applying default settings and overrides, making the codebase more maintainable.
83-103: LGTM! CLI-driven SDK generation is well-implemented.The conditional generation flow allows selective SDK builds via command-line arguments while maintaining backward compatibility when no arguments are provided. The runtime message provides helpful feedback when generating a specific SDK.
106-280: LGTM! SDK generation blocks now consistently use the configureSDK helper.Most SDK blocks (PHP, Web, Node, CLI, Ruby, Python, Dart, Flutter, React Native, Go, Swift, Apple, DotNet, REST, Android, Kotlin) now call
configureSDK, reducing duplication. The conditional wrapping enables selective generation while preserving the default "generate all" behavior.Note: The Android and Kotlin namespace issues flagged in previous reviews have been correctly fixed to use
'io.appwrite'(lines 253, 270).
ChiragAgg5k
force-pushed
the
feat-sdk-build-validation-workflow
branch
from
92ed13c to
e0d00a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
example.php (1)
265-269: Verify that namespace is set for Kotlin SDK.Similar to Android, a previous review flagged that the Kotlin SDK namespace
'io appwrite'was invalid (space instead of dot). The current code doesn't set a namespace viaconfigureSDKoverrides, and the function defaults don't include one.Verify that the
Kotlinlanguage class provides a default namespace. If not, you may need to add a namespace override to theconfigureSDKcall.Run the following script to check if the Kotlin class has a default namespace:
#!/bin/bash # Description: Check if Kotlin language class has a default namespace configuration. # Search for namespace in Kotlin class rg -n -A5 -B5 'namespace' src/SDK/Language/Kotlin.php # Check class definition and params rg -n -A10 'class Kotlin' src/SDK/Language/Kotlin.php
🧹 Nitpick comments (2)
example.php (2)
39-90: Consider adding type hints and PHPDoc for clarity.The
configureSDKhelper effectively centralizes common configuration and reduces duplication. To improve maintainability, consider adding type hints and PHPDoc:+/** + * Configure an SDK instance with default and optional override settings. + * + * @param SDK $sdk The SDK instance to configure + * @param array<string, mixed> $overrides Optional configuration overrides + * @return SDK The configured SDK instance + */ -function configureSDK($sdk, $overrides = []) { +function configureSDK(SDK $sdk, array $overrides = []): SDK {
272-276: Provide GraphQL-specific configuration overrides.A previous review requested that GraphQL use
configureSDKfor consistency and suggested providing specific values for name, description, and logo. The current code usesconfigureSDK(good), but with no overrides, GraphQL will receive default placeholder values (name='NAME', generic description, default logo).Consider passing GraphQL-specific overrides to match the intent of the previous review:
if (!$requestedSdk || $requestedSdk === 'graphql') { $sdk = new SDK(new GraphQL(), new Swagger2($spec)); - configureSDK($sdk); + configureSDK($sdk, [ + 'name' => 'GraphQL', + 'description' => 'Appwrite is an open-source backend as a service server that abstract and simplify complex and repetitive development tasks behind a very simple to use REST API. Appwrite aims to help you develop your apps faster and in a more secure way. Use the Flutter SDK to integrate your app with the Appwrite server to easily start interacting with all of Appwrite backend APIs and tools. For full API documentation and tutorials go to https://appwrite.io/docs', + ]); $sdk->generate(__DIR__ . '/examples/graphql'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (13)
src/SDK/SDK.php (21)
setName(257-262)setVersion(290-295)setDescription(268-273)setShortDescription(279-284)setURL(389-394)setLogo(378-383)setLicenseContent(323-328)setWarning(444-449)setChangelog(477-482)setGitUserName(356-361)setGitRepoName(345-350)setTwitter(512-517)setDiscord(500-506)setDefaultHeaders(235-240)setReadme(466-471)setExamples(488-493)setLicense(312-317)setNamespace(246-251)setExclude(750-759)SDK(21-938)generate(618-737)src/SDK/Language/PHP.php (3)
PHP(8-447)setComposerVendor(22-27)setComposerPackage(33-38)src/SDK/Language/Node.php (1)
Node(5-252)src/SDK/Language/CLI.php (3)
CLI(8-455)setExecutableName(80-85)setLogoUnescaped(102-107)src/SDK/Language/JS.php (1)
setNPMPackage(19-24)src/SDK/Language/Dart.php (2)
Dart(8-508)setPackageName(21-26)src/SDK/Language/Flutter.php (1)
Flutter(5-360)src/SDK/Language/ReactNative.php (1)
ReactNative(7-256)src/SDK/Language/Go.php (1)
Go(8-372)src/SDK/Language/Swift.php (1)
Swift(8-620)src/SDK/Language/Apple.php (1)
Apple(5-496)src/SDK/Language/REST.php (1)
REST(5-135)src/SDK/Language/GraphQL.php (1)
GraphQL(5-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: node (server)
- GitHub Check: apple (client)
- GitHub Check: android (client)
- GitHub Check: web (client)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, Go118)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Android14Java17)
🔇 Additional comments (2)
example.php (2)
92-112: LGTM!The CLI argument parsing correctly enables per-SDK generation and defaults the platform to 'console' as intended. The runtime feedback message is helpful for tracking execution.
115-276: LGTM! Conditional generation pattern is well-implemented.The conditional wrapping of each SDK generation block (
if (!$requestedSdk || $requestedSdk === '...')) correctly enables per-SDK generation as described in the PR objectives. The pattern is consistent across all SDKs, and language-specific configurations are appropriately applied before callingconfigureSDK. The use ofconfigureSDKeffectively reduces code duplication while allowing SDK-specific overrides where needed (CLI exclusions, React Native examples/version, Go git settings).
This commit introduces a GitHub Actions workflow that generates and validates SDKs using the real Appwrite spec files instead of the generic test spec.
Supported SDKs:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests