-
Notifications
You must be signed in to change notification settings - Fork 190
fix: print sdk warnings to stderr instead of stdout #1200
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
Conversation
WalkthroughThis change updates multiple client templates to emit HTTP response warning messages to standard error instead of standard output. The affected templates are: Android (Kotlin) library, Apple Swift, .NET C#, Kotlin, Python, and Swift. Implementations replace stdout printing with stderr equivalents (e.g., System.err, Console.Error, fputs to stderr, sys.stderr). The Python template adds an import of sys for stderr usage. No public/exported API signatures are modified; control flow and response handling logic remain the same. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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.
Pull Request Overview
This PR fixes SDK warning messages to output to stderr instead of stdout across multiple platform client templates. This change ensures that warning messages don't interfere with standard program output, which is a best practice for diagnostic messages.
Key changes:
- Updated warning output methods in client templates for Swift, Python, Kotlin, .NET, and Android
- Added necessary imports where required (sys module in Python)
- Maintained consistent warning message format while redirecting output stream
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
templates/swift/Sources/Client.swift.twig | Changed from print() to fputs() with stderr |
templates/python/package/client.py.twig | Added sys import and updated print to use stderr |
templates/kotlin/src/main/kotlin/io/appwrite/Client.kt.twig | Changed from println() to System.err.println() |
templates/dotnet/Package/Client.cs.twig | Changed from Console.WriteLine() to Console.Error.WriteLine() |
templates/apple/Sources/Client.swift.twig | Changed from print() to fputs() with stderr |
templates/android/library/src/main/java/io/package/Client.kt.twig | Changed from println() to System.err.println() |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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 (4)
templates/swift/Sources/Client.swift.twig (1)
353-354
: LGTM; optional: prefer FileHandle.standardError for portability.Avoids Darwin/Glibc imports on some platforms. Equivalent behavior:
- fputs("Warning: \(warning)\n", stderr) + FileHandle.standardError.write(Data("Warning: \(warning)\n".utf8))templates/python/package/client.py.twig (1)
99-103
: LGTM; minor nit: trim warning fragments.Safer if the header contains spaces around semicolons:
- print(f'Warning: {warning}', file=sys.stderr) + print(f'Warning: {warning.strip()}', file=sys.stderr)templates/dotnet/Package/Client.cs.twig (1)
292-298
: Split multi-warning header values for parity with other SDKs (optional).Current code prints the whole header value if multiple warnings are semicolon-separated. For consistency:
- if (response.Headers.TryGetValues("x-{{ spec.title | lower }}-warning", out var warnings)) - { - foreach (var warning in warnings) - { - Console.Error.WriteLine("Warning: " + warning); - } - } + if (response.Headers.TryGetValues("x-{{ spec.title | lower }}-warning", out var warnings)) + { + foreach (var headerValue in warnings) + { + foreach (var warning in headerValue.Split(';', StringSplitOptions.RemoveEmptyEntries)) + { + Console.Error.WriteLine("Warning: " + warning.Trim()); + } + } + }templates/apple/Sources/Client.swift.twig (1)
305-309
: LGTM; optional: prefer FileHandle.standardError for portability.Equivalent change:
- fputs("Warning: \(warning)\n", stderr) + FileHandle.standardError.write(Data("Warning: \(warning)\n".utf8))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
templates/android/library/src/main/java/io/package/Client.kt.twig
(1 hunks)templates/apple/Sources/Client.swift.twig
(1 hunks)templates/dotnet/Package/Client.cs.twig
(1 hunks)templates/kotlin/src/main/kotlin/io/appwrite/Client.kt.twig
(1 hunks)templates/python/package/client.py.twig
(2 hunks)templates/swift/Sources/Client.swift.twig
(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). (15)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Python310)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, Android14Java17)
🔇 Additional comments (3)
templates/python/package/client.py.twig (1)
5-5
: LGTM: sys import is needed for stderr.templates/kotlin/src/main/kotlin/io/appwrite/Client.kt.twig (1)
556-556
: LGTM: warnings now go to stderr.templates/android/library/src/main/java/io/package/Client.kt.twig (1)
510-511
: LGTM: Android now writes warnings to stderr.
What does this PR do?
fixes the stdout issue
Test Plan
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
yes.
Summary by CodeRabbit