-
-
Notifications
You must be signed in to change notification settings - Fork 28
Removed robots.txt file in favour of a module and show all errors in local and CI.
#1964
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
|
Warning Rate limit exceeded@AlexSkrypnyk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 15 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 ignored due to path filters (30)
📒 Files selected for processing (9)
WalkthroughUpdates Drupal configuration to exclude scaffolding of robots.txt, removes the existing robots.txt file, expands environment-based settings to show all errors and skip permissions hardening for local and CI, and adjusts PHPUnit environment tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Drupal as Drupal Bootstrap
participant Settings as settings.system.php
participant Config as Drupal Config
User->>Drupal: HTTP request
Drupal->>Settings: Load environment settings
alt ENVIRONMENT_LOCAL or ENVIRONMENT_CI
Settings->>Config: Set system.logging.error_level = "all"
Settings->>Drupal: Set skip_permissions_hardening = TRUE
else Other environments
Settings-->>Drupal: No changes
end
Drupal-->>User: Response (logging level applied)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (23)
.vortex/installer/tests/Fixtures/install/_baseline/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/robots.txtis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/modules/settings.system.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/robots.txtis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/modules/settings.system.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/web/-robots.txtis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/starter_drupal_cms_profile/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_groups_no_be_lint_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_no_behat_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_no_phpcs_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_no_phpmd_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_no_phpstan_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/tools_no_rector_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
composer.json(1 hunks)tests/phpunit/Drupal/EnvironmentSettingsTest.php(6 hunks)web/robots.txt(0 hunks)web/sites/default/includes/modules/settings.system.php(1 hunks)
💤 Files with no reviewable changes (1)
- web/robots.txt
🧰 Additional context used
📓 Path-based instructions (1)
composer.json
📄 CodeRabbit inference engine (CLAUDE.md)
When adding JavaScript or CSS libraries as Drupal libraries with Composer, define them as inline Composer packages in the 'repositories' section of composer.json
Files:
composer.json
⏰ 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). (7)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (6)
composer.json (1)
109-109: Verify robotstxt module configuration
- Scaffolding exclusion (
"[web-root]/robots.txt": false) is present; no static robots.txt under web/- Unable to find any
core.extension.ymlin the repo to confirm the robotstxt module is enabled—please verify it’s listed under enabled modules in your Drupal config- No server configs explicitly handling robots.txt were detected under docker/ or web/.htaccess
web/sites/default/includes/modules/settings.system.php (1)
22-23: Enable verbose errors for Local/CI — LGTM.Matches the PR goal to surface all errors in dev and CI.
tests/phpunit/Drupal/EnvironmentSettingsTest.php (4)
425-425: Local: assertions updated — LGTM.Config adds system.logging.error_level = 'all' and settings add skip_permissions_hardening = TRUE; ordering remains alphabetical.
Also applies to: 445-445
472-472: Local (container): assertions updated — LGTM.Mirrors Local expectations with correct alphabetic placement.
Also applies to: 492-492
522-522: CircleCI: show all errors — LGTM.Config matches settings change for CI.
571-571: GitHub Actions: show all errors — LGTM.Consistent with CI expectations.
| } | ||
|
|
||
| if ($settings['environment'] === ENVIRONMENT_CI) { | ||
| if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) { |
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.
🧹 Nitpick (assertive)
Nit: simplify environment check.
Use in_array for readability and to avoid repeating the variable.
-if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) {
+if (in_array($settings['environment'], [ENVIRONMENT_LOCAL, ENVIRONMENT_CI], TRUE)) {📝 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.
| if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) { | |
| if (in_array($settings['environment'], [ENVIRONMENT_LOCAL, ENVIRONMENT_CI], TRUE)) { |
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.system.php around line 19, the
environment comparison repeats the same variable and can be simplified; replace
the dual === checks with a single in_array($settings['environment'],
[ENVIRONMENT_LOCAL, ENVIRONMENT_CI]) call to improve readability and avoid
repeating $settings['environment'].
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1964 +/- ##
========================================
Coverage 77.57% 77.57%
========================================
Files 89 89
Lines 5515 5515
Branches 35 35
========================================
Hits 4278 4278
Misses 1237 1237 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
281d16c to
ac32cc1
Compare
ac32cc1 to
40576d6
Compare
related to #1915
Summary by CodeRabbit
Chores
Tests
Refactor