Skip to content

update sdks#10729

Merged
abnegate merged 4 commits into1.8.xfrom
update-sdks-2
Oct 31, 2025
Merged

update sdks#10729
abnegate merged 4 commits into1.8.xfrom
update-sdks-2

Conversation

@ChiragAgg5k
Copy link
Member

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

📝 Walkthrough

Walkthrough

This PR updates SDK version literals in app/config/platforms.php:

  • Flutter: 20.2.1 → 20.2.2
  • Apple: 13.3.0 → 13.3.1
  • Command Line (CLI): 10.2.3 → 11.0.0
  • Python (Server): 13.4.1 → 13.5.0

It also extends src/Appwrite/Platform/Tasks/SDKs.php by adding a new CLI option/parameter sdks (and the corresponding action parameter), enabling non‑interactive selective SDK generation, and replacing the existing PR edit flow with a head-based gh pr list lookup plus gh api PATCH to update PRs; error handling and logging around PR retrieval/updates were added.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Heterogeneous changes: small config literal updates plus substantive behavioral/CLI changes in one task class.
  • Areas needing extra attention:
    • src/Appwrite/Platform/Tasks/SDKs.php — CLI option registration, updated action signature, parsing of the new sdks parameter, conditional selection logic, and robustness of gh command + GitHub API calls and error handling.
    • app/config/platforms.php — verify version strings and ensure no accidental structural changes.
    • Any entrypoints/tests invoking the SDKs task — ensure compatibility with the new action signature.

Possibly related PRs

Suggested reviewers

  • abnegate
  • loks0n
  • stnguyen90

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "update sdks" is extremely vague and generic, using a non-descriptive term that fails to convey meaningful information about the specific changes in the pull request. While the changeset does involve SDKs, the actual modifications span from version bumps in app/config/platforms.php to significant functionality additions in src/Appwrite/Platform/Tasks/SDKs.php (including a new CLI parameter and PR update logic). The title does not clarify what type of SDK updates were made or their scope.
Description Check ❓ Inconclusive No pull request description was provided by the author. Since the description is completely absent, it conveys no information about the changeset and fails to satisfy the pass criterion of being "related in some way to the changeset." While the check is designed to be lenient, a missing description that provides zero context about the changes falls into the category of being extremely vague and generic. Add a description that explains the purpose and scope of the changes, such as: what SDK versions were updated and why, what the new batch SDK generation feature accomplishes, and what problem the PR update logic changes address. This helps reviewers understand the intent behind the modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-sdks-2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 29, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.44-r2 CVE-2025-5244 HIGH
binutils 2.44-r2 CVE-2025-5245 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
pcre2 10.43-r1 CVE-2025-58050 CRITICAL
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH
stdlib 1.22.10 CVE-2025-47912 HIGH
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58185 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH
stdlib 1.22.10 CVE-2025-58188 HIGH
stdlib 1.22.10 CVE-2025-58189 HIGH
stdlib 1.22.10 CVE-2025-61723 HIGH
stdlib 1.22.10 CVE-2025-61724 HIGH
stdlib 1.22.10 CVE-2025-61725 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6072675 and baa7a17.

⛔ Files ignored due to path filters (5)
  • composer.lock is excluded by !**/*.lock
  • docs/sdks/apple/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/cli/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/flutter/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/python/CHANGELOG.md is excluded by !docs/sdks/**
📒 Files selected for processing (1)
  • app/config/platforms.php (4 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). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan

@github-actions
Copy link

github-actions bot commented Oct 29, 2025

✨ Benchmark results

  • Requests per second: 1,128
  • Requests with 200 status code: 203,003
  • P99 latency: 0.171214744

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,128 1,173
200 203,003 211,186
P99 0.171214744 0.176767833

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between baa7a17 and 88dffcf.

⛔ Files ignored due to path filters (7)
  • composer.lock is excluded by !**/*.lock
  • docs/examples/1.8.x/console-cli/examples/migrations/create-csv-export.md is excluded by !docs/examples/**
  • docs/examples/1.8.x/console-cli/examples/migrations/create-csv-import.md is excluded by !docs/examples/**
  • docs/sdks/apple/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/cli/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/flutter/CHANGELOG.md is excluded by !docs/sdks/**
  • docs/sdks/python/CHANGELOG.md is excluded by !docs/sdks/**
📒 Files selected for processing (2)
  • app/config/platforms.php (4 hunks)
  • src/Appwrite/Platform/Tasks/SDKs.php (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/config/platforms.php
⏰ 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: E2E Service Test (VCS)
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (Tokens)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (Teams)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: Unit Test
  • GitHub Check: E2E General Test
  • GitHub Check: Benchmark
  • GitHub Check: scan

Comment on lines +57 to +62
if (!$sdks) {
$selectedPlatform ??= Console::confirm('Choose Platform ("' . APP_PLATFORM_CLIENT . '", "' . APP_PLATFORM_SERVER . '", "' . APP_PLATFORM_CONSOLE . '" or "*" for all):');
$selectedSDK ??= \strtolower(Console::confirm('Choose SDK ("*" for all):'));
} else {
$sdks = explode(',', $sdks);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize --sdks input so filtered runs actually execute

Right now we keep raw tokens from explode(',', $sdks). If a user runs --sdks cli, python (note the space) or uses capitalisation (--sdks Flutter), the resulting entries are ' python'/'Flutter'. Both fail the subsequent in_array($language['key'], $sdks) check because the platform keys are lowercase without leading spaces, so nothing gets generated. The interactive path still lowercases via strtolower, so this is a regression for the new CLI flag. Please trim and lowercase the parsed tokens (and drop empties) before using them.

-            $sdks = explode(',', $sdks);
+            $sdks = array_values(array_filter(array_map(
+                static fn(string $sdk): string => strtolower(trim($sdk)),
+                explode(',', $sdks)
+            )));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!$sdks) {
$selectedPlatform ??= Console::confirm('Choose Platform ("' . APP_PLATFORM_CLIENT . '", "' . APP_PLATFORM_SERVER . '", "' . APP_PLATFORM_CONSOLE . '" or "*" for all):');
$selectedSDK ??= \strtolower(Console::confirm('Choose SDK ("*" for all):'));
} else {
$sdks = explode(',', $sdks);
}
if (!$sdks) {
$selectedPlatform ??= Console::confirm('Choose Platform ("' . APP_PLATFORM_CLIENT . '", "' . APP_PLATFORM_SERVER . '", "' . APP_PLATFORM_CONSOLE . '" or "*" for all):');
$selectedSDK ??= \strtolower(Console::confirm('Choose SDK ("*" for all):'));
} else {
$sdks = array_values(array_filter(array_map(
static fn(string $sdk): string => strtolower(trim($sdk)),
explode(',', $sdks)
)));
}
🤖 Prompt for AI Agents
In src/Appwrite/Platform/Tasks/SDKs.php around lines 57 to 62, the parsed --sdks
tokens from explode(',', $sdks) are not normalized so values with spaces or
capitalization won't match platform keys; replace the current explode with a
pipeline that explodes on comma, trims each token, lowercases each token, and
filters out empty strings (e.g. explode -> array_map('trim', ...) ->
array_map('strtolower', ...) -> array_filter(...)), then assign the resulting
array back to $sdks so subsequent in_array checks work correctly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88dffcf and 6bd5287.

⛔ Files ignored due to path filters (1)
  • docs/sdks/cli/CHANGELOG.md is excluded by !docs/sdks/**
📒 Files selected for processing (1)
  • app/config/platforms.php (4 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). (20)
  • GitHub Check: Benchmark
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Migrations)
  • GitHub Check: E2E Service Test (Tokens)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E General Test
  • GitHub Check: scan

@abnegate abnegate merged commit 8ca083e into 1.8.x Oct 31, 2025
71 of 72 checks passed
@abnegate abnegate deleted the update-sdks-2 branch October 31, 2025 06:54
@coderabbitai coderabbitai bot mentioned this pull request Nov 3, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants