Skip to content

fix: remove storage service from include_once#11043

Merged
ChiragAgg5k merged 2 commits into1.8.xfrom
fix-include-services
Dec 30, 2025
Merged

fix: remove storage service from include_once#11043
ChiragAgg5k merged 2 commits into1.8.xfrom
fix-include-services

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

@ChiragAgg5k ChiragAgg5k commented Dec 30, 2025

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 30, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The controller property of the Storage service in app/config/services.php is changed from 'api/storage.php' to an empty string. This modification alters how the storage service's controller path is configured, potentially shifting from an explicit file reference to an alternative routing mechanism.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title mentions removing the storage service from include_once, but the actual change only modifies the controller path from 'api/storage.php' to an empty string in the service configuration. The title is misleading - it claims to remove storage service from include_once but the change only clears the controller field. Update the title to accurately reflect that the controller path is being removed or cleared.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to verify if there is any explanation for the changes. Add a detailed description explaining why the storage service controller path is being cleared and what the intended behavior change is.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent 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 c4fcec0 and d8d3f22.

📒 Files selected for processing (1)
  • app/config/services.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 (1)
app/config/services.php (1)

149-149: Add missing explanatory comment for consistency.

The change removes the explicit controller path for the storage service, which is correct for module-based routing. However, this line should include the // Uses modules comment like the other services at lines 65, 79, 205, 219, and 233 for code consistency.

-        'controller' => '',
+        'controller' => '', // Uses modules

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.

@ChiragAgg5k ChiragAgg5k changed the title Fix: robust SMTP validation and added regression test fix: remove storage service from include_once Dec 30, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 30, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 30, 2025

✨ Benchmark results

  • Requests per second: 1,174
  • Requests with 200 status code: 211,281
  • P99 latency: 0.165307552

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,174 1,247
200 211,281 224,564
P99 0.165307552 0.16131723

@ChiragAgg5k ChiragAgg5k merged commit 57e2182 into 1.8.x Dec 30, 2025
40 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix-include-services branch December 30, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lohanidamodar lohanidamodar lohanidamodar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChiragAgg5k @lohanidamodar