chore: remove unused + bump dev dependencies #380
Conversation
|
Warning Rate limit exceeded@Chris53897 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 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)
WalkthroughRemoved the Geoip provider factory and its test config, updated phpstan baseline paths/ignore rules, removed provider entries from docs, and bumped/reshuffled multiple dev geocoder-related dependencies in composer.json. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composer.json (1)
25-70: Remove orphaned geoip/geoip references from tests/bootstrap.php.The
geoip/geoippackage was removed from composer.json buttests/bootstrap.phpstill contains require statements for its files:
- Line:
require_once __DIR__.'/../vendor/geoip/geoip/src/geoip.inc';- Line:
require_once __DIR__.'/../vendor/geoip/geoip/src/geoipcity.inc';These will cause test failures since the package no longer exists. Remove these require statements from the bootstrap file. The other two removed packages (
geocoder-php/geoips-providerandgeocoder-php/mapzen-provider) have no remaining references.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
composer.json(1 hunks)
🔇 Additional comments (1)
composer.json (1)
41-45: Dev dependency version bumps look good.The version increments are consistent minor/patch bumps across all geocoder providers, aligning with the goal of supporting
willdurand/geocoder5.0.0. Thenyholm/psr7bump to ^1.8 is also a reasonable maintenance update.Also applies to: 49-56, 58-58
| "fakerphp/faker": "^1.20", | ||
| "friendsofphp/php-cs-fixer": "^3.13", | ||
| "geocoder-php/algolia-places-provider": "^0.4", | ||
| "geocoder-php/algolia-places-provider": "^0.5", |
There was a problem hiding this comment.
Algolia Places API is no longer functional.
The Algolia Places API shut down on May 31st, 2022. While the geocoder-php/algolia-places-provider package still exists and receives maintenance updates, the underlying service is non-functional. If this provider is retained solely for testing the bundle's integration patterns, this is acceptable. Otherwise, this dependency should be removed and replaced with an alternative geocoding provider.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
doc/services.md(0 hunks)phpstan-baseline.php(1 hunks)src/ProviderFactory/GeoipFactory.php(0 hunks)tests/Functional/config/provider/geoip.yml(0 hunks)
💤 Files with no reviewable changes (3)
- doc/services.md
- src/ProviderFactory/GeoipFactory.php
- tests/Functional/config/provider/geoip.yml
🧰 Additional context used
🪛 GitHub Check: PHP-CS-Fixer
phpstan-baseline.php
[warning] 1-1:
Found violation(s) of type: declare_equal_normalize
[warning] 1-1:
Found violation(s) of type: concat_space
[warning] 1-1:
Found violation(s) of type: blank_line_after_opening_tag
🔇 Additional comments (3)
phpstan-baseline.php (3)
8-194: LGTM: Path formatting standardized.The path concatenation formatting has been consistently updated throughout the file to use spaces around the concatenation operator (
__DIR__ . '/path'), which aligns with theconcat_spacecoding standard and improves readability.
176-194: GeoipFactory to GeoIP2Factory migration verified as complete.The phpstan-baseline.php updates correctly reflect the removal of the old Geoip provider. GeoipFactory.php has been completely removed from the codebase, all factory references have been properly migrated to GeoIP2Factory, and configuration files and documentation correctly reference the updated factory. The baseline error path updates are appropriate and the changes are ready to merge.
22-27: This PHPStan suppression is appropriate. The sprintf call at lines 81-85 passes a potentially mixed-type value (is_array($value) ? json_encode($value) : $value) as the second argument after the format string. While PHPStan strictly requiresbool|float|int|string|null, sprintf handles type coercion at runtime, and the code functions correctly. Suppressing this single instance is more pragmatic than adding explicit type casting for display output.
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| <?php declare(strict_types = 1); |
There was a problem hiding this comment.
Fix the declare statement formatting.
The space around = in the declare statement violates the declare_equal_normalize rule. According to PHP-CS-Fixer standards, there should be no spaces around the equals sign.
🔎 Proposed fix
-<?php declare(strict_types = 1);
+<?php declare(strict_types=1);Based on static analysis hints.
📝 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.
| <?php declare(strict_types = 1); | |
| <?php declare(strict_types=1); |
🧰 Tools
🪛 GitHub Check: PHP-CS-Fixer
[warning] 1-1:
Found violation(s) of type: declare_equal_normalize
[warning] 1-1:
Found violation(s) of type: concat_space
[warning] 1-1:
Found violation(s) of type: blank_line_after_opening_tag
🤖 Prompt for AI Agents
In phpstan-baseline.php around line 1, the declare statement has spaces around
the equals sign; update the declaration to remove the spaces so it reads
declare(strict_types=1); to comply with the declare_equal_normalize PHP-CS-Fixer
rule and PHP formatting standards.
|
I have regenerated the baseline (PHP 8.5). This conflicts with PHP-CS-Fixer. I excluded the baseline from PHP-CS-Fixer |
|
A new Release is needed for BC reasons. But i am pretty sure, you are aware of this. |
2 participants
removed in #369
this is required for
Upgrading willdurand/geocoder (4.6.0 => 5.0.0)Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.