chore: add more sdk config flexibility#11037
Conversation
📝 WalkthroughWalkthroughThis pull request modifies SDK metadata generation in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Appwrite/Platform/Tasks/SDKs.php (1)
141-142: Consider applying consistent path resolution to overrides.The fallback path uses
realpath()to resolve the path, but the override$language['gettingStarted']doesn't. This means override paths won't be canonicalized, which could lead to inconsistent path handling if relative paths are provided.Consider applying
realpath()to the override as well to ensure consistent behavior:🔎 Proposed fix for consistent path handling
-$gettingStarted = $language['gettingStarted'] ?? \realpath(__DIR__ . '/../../../../docs/sdks/' . $language['key'] . '/GETTING_STARTED.md'); +$gettingStarted = isset($language['gettingStarted']) ? \realpath($language['gettingStarted']) : \realpath(__DIR__ . '/../../../../docs/sdks/' . $language['key'] . '/GETTING_STARTED.md');
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Platform/Tasks/SDKs.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). (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (3)
src/Appwrite/Platform/Tasks/SDKs.php (3)
1-1: Verify PR title and changes alignment.The PR title mentions "Fix: robust SMTP validation and added regression test" but the code changes are about SDK generation configuration (language-specific
gettingStartedpaths andshortDescriptionoverrides). There's no SMTP-related code in this diff.Please confirm:
- Are these the correct changes for this PR?
- Should the PR title be updated to reflect the actual changes?
384-384: LGTM!The language-specific
shortDescriptionoverride with fallback to the default value is implemented correctly and follows the same pattern as the description field on line 383.
141-141: No action needed. ThegettingStartedandshortDescriptionfields are optional configuration parameters with sensible default fallbacks. The code properly handles missing values using the null coalescing operator (??), so the feature works as intended without requiring these fields to be explicitly defined in the SDK configuration.
ChiragAgg5k
changed the title
✨ Benchmark results
⚡ Benchmark Comparison
|
2 participants
No description provided.