-
Notifications
You must be signed in to change notification settings - Fork 35
feat: Apple SDK update for version 13.5.0 #96
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
|
Warning Rate limit exceeded@ChiragAgg5k has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 42 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)
WalkthroughThis PR introduces version 13.5.0 of the SDK, adding a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Sources/AppwriteEnums/Theme.swift (1)
3-5: Consider omitting redundant string enum values.SwiftLint correctly identifies that explicit raw values can be omitted when they match the case name. However, since this SDK is auto-generated, maintaining explicit values may be intentional for clarity or generator consistency.
-public enum Theme: String, CustomStringConvertible { - case light = "light" - case dark = "dark" +public enum Theme: String, CustomStringConvertible { + case light + case darkSources/Appwrite/Services/Avatars.swift (1)
293-382: NewgetScreenshotendpoint wiring looks consistent with existing Avatars APIsThe overall shape of the method (path construction,
apiParams, inclusion ofproject, andGETcall) is consistent with the rest of the service and should integrate cleanly. Using the new enums (Theme,Timezone,Output) directly inapiParamsalso aligns with theCustomStringConvertiblepattern introduced for them.Two minor improvement points:
Tighten the
headerstype and docs if possibleExposing
headersasAny?in a public API makes it hard for callers to know what shape is actually expected and can hide misuse until runtime (depending on howparametersToQueryStringencodes values). If the backend expects a concrete structure (e.g.[String: String], or an array of{ name, value }objects), it would be better to:
- Use a more specific Swift type instead of
Any?, and- Update the doc comment from “
headers: Any (optional)” to describe the exact expected format.That keeps this API more discoverable and type‑safe, and also ensures whatever encoding the client does for GET query parameters matches what the backend expects.
(Optional) Consider an options type for the growing parameter list
This method now has a long list of optional, mostly configuration-like parameters. For future major versions (or if the generator allows), consider introducing something like
ScreenshotOptionsand accepting a single options value instead of many positional arguments. That would make call sites more readable and easier to extend without constantly expanding the signature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)README.md(1 hunks)Sources/Appwrite/Client.swift(2 hunks)Sources/Appwrite/Services/Avatars.swift(1 hunks)Sources/AppwriteEnums/Output.swift(1 hunks)Sources/AppwriteEnums/Theme.swift(1 hunks)Sources/AppwriteEnums/Timezone.swift(1 hunks)docs/examples/avatars/get-screenshot.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/Appwrite/Services/Avatars.swift (1)
Sources/Appwrite/Client.swift (1)
call(302-333)
🪛 markdownlint-cli2 (0.18.1)
docs/examples/avatars/get-screenshot.md
11-11: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
Sources/AppwriteEnums/Theme.swift
[Warning] 4-4: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
[Warning] 5-5: String enum values can be omitted when they are equal to the enumcase name
(redundant_string_enum_value)
🔇 Additional comments (7)
README.md (1)
34-34: Version update looks good.The Swift Package Manager dependency version is correctly updated to 13.5.0, consistent with the SDK version in Client.swift and the CHANGELOG entry.
CHANGELOG.md (1)
3-5: LGTM!The changelog entry correctly documents the new
getScreenshotmethod addition.Sources/Appwrite/Client.swift (2)
8-8: Public re-export of JSONCodable added.Using
@_exportedmakes allJSONCodablesymbols publicly available to consumers of this SDK without requiring them to explicitly import it. This is a public API surface expansion.Verify this is intentional - consumers will now have implicit access to JSONCodable types. If JSONCodable is only used internally, a regular
importwould be more appropriate to avoid polluting the consumer's namespace.
27-27: Version header updated correctly.The SDK version header is properly bumped to 13.5.0.
Sources/AppwriteEnums/Output.swift (1)
1-15: LGTM!The
Outputenum correctly defines supported image formats for the screenshot API. The inclusion of bothjpgandjpegcases likely reflects API compatibility requirements.docs/examples/avatars/get-screenshot.md (1)
10-31: Example documentation looks complete.The example demonstrates all available parameters for
getScreenshot. The markdownlint warning about the bare URL on line 11 is a false positive since it's a code parameter value within a code block.Some parameter values appear to be boundary/edge cases (e.g.,
scale: 0.1,quality: -1,width: 0,height: 0). Verify these are valid API values or consider using more realistic example values that users would typically specify.Sources/AppwriteEnums/Timezone.swift (1)
3-426: I'll help you verify the timezone format concern. Let me search the codebase to understand if the lowercase format is intentional or if it needs to match standard IANA conventions.
<function_calls>
#!/bin/bashSearch for timezone-related code and usage patterns
find . -type f -name "*.swift" | head -20
</function_calls>Let me search more specifically for timezone-related code and API usage:
<function_calls>
#!/bin/bashLook for files that reference timezone or Timezone
rg -l "Timezone|timezone" --type swift | head -20
</function_calls>Now let me check how timezones are actually used in API calls and if there's any documentation about the format:
<function_calls>
#!/bin/bashSearch for usage of the Timezone enum
rg "Timezone." --type swift -C 2 | head -50
</function_calls>Let me also check if there are any tests or examples showing timezone format:
<function_calls>
#!/bin/bashLook for timezone format in test files or examples
rg "africa|america|europe|asia|pacific|atlantic|indian|antarctica|arctic" --type swift -i | grep -i "timezone|rawvalue" | head -20
</function_calls>Let me search for documentation files or README that might explain the timezone format:
<function_calls>
#!/bin/bashCheck for documentation files
find . -type f ( -name "README*" -o -name ".md" -o -name ".txt" ) | head -20
</function_calls>Let me check the git history to see if there are any comments about why this format was chosen:
<function_calls>
#!/bin/bashLook at the Timezone.swift file structure and comments
head -100 Sources/AppwriteEnums/Timezone.swift
</function_calls>Let me verify the actual raw values more carefully:
<function_calls>
#!/bin/bashExtract and analyze the raw values to confirm the format pattern
grep '= "' Sources/AppwriteEnums/Timezone.swift | head -30
</function_calls>Let me search for any backend API code that might give hints about expected timezone format:
<function_calls>
#!/bin/bashLook for API or networking code
find . -type f -name "*.swift" | xargs grep -l "request|api|client" -i | head -10
</function_calls>Let me check if there's a similar enum or pattern in other Appwrite SDK implementations:
<function_calls>
#!/bin/bashLook for other enum files to understand the pattern
find . -type f -name "Enum.swift" -o -name "enum.swift" | head -10
</function_calls>Let me look for API documentation or specs:
<function_calls>
#!/bin/bashSearch for timezone format references in any documentation
rg "timezone" --type md -i -C 2
</function_calls>Let me check one more thing - look at the directory structure and see if there's any related documentation:
<function_calls>
#!/bin/bashCheck directory structure and look for documentation
ls -la | head -20
find . -maxdepth 2 -type f -name ".md" -o -name ".txt"
</function_calls>Let me verify the format one more time and check for any variations:
<function_calls>
#!/bin/bashCount and verify the timezone format consistency
grep 'case ' Sources/AppwriteEnums/Timezone.swift | wc -l
Check if all use lowercase with forward slashes
grep 'case .* = "[a-z_/]*"' Sources/AppwriteEnums/Timezone.swift | wc -l
Look for any that don't match the pattern
grep 'case ' Sources/AppwriteEnums/Timezone.swift | grep -v '= "[a-z_/]*"' | head -10
</function_calls>Let me search for any references to IANA or timezone standards:
<function_calls>
#!/bin/bashSearch for IANA, timezone database, or similar references
rg "IANA|zone|database|format|standard" --type swift -i | grep -i "timezone|tz" | head -20
</function_calls>Let me check if this is an Appwrite SDK and look for backend integration:
<function_calls>
#!/bin/bashLook for Appwrite-specific files or references
find . -type f -name "*.swift" | xargs grep -l "appwrite" -i | head -10
</function_calls>Let me check the git log for the Timezone.swift file to see if there's context about why this format was chosen:
<function_calls>
#!/bin/bashCheck git log for Timezone.swift
git log --oneline Sources/AppwriteEnums/Timezone.swift | head -10
</function_calls>Let me verify the complete list of values to see the format:
<function_calls>
#!/bin/bashShow all timezone values with their format
grep '= "' Sources/AppwriteEnums/Timezone.swift | sed 's/.= "(.)".*/\1/' | sort | uniq -c | head -30
</function_calls>Now let me check if there are test files that validate this format:
<function_calls>
#!/bin/bashLook for test files
find . -type f -name "Test" -o -name "test" -o -name "spec" | grep -i swift
</function_calls>Let me verify what the standard IANA timezone format actually is and if the lowercase version is acceptable:
<function_calls>
IANA timezone database format standard Africa/Addis_Ababa vs africa/addis_ababa case sensitivity
</function_calls>
This PR contains updates to the Apple SDK for version 13.5.0.