Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough## Walkthrough
The continuous integration workflow was refactored to expand matrix testing across multiple operating systems and PHP versions, update action versions, improve dependency management, and modernize configuration. Job naming, environment variables, and CI steps were clarified. Redis and Memcached services setup and demo script execution were revised, with more precise triggers and improved caching.
## Changes
| File(s) | Change Summary |
|------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|
| .github/workflows/continuous-integration.yml | Refactored workflow: expanded matrix (OS, PHP, dependencies), updated action versions, revised env vars, improved caching, clarified steps, modernized triggers, updated Redis and Memcached service setup, and revised demo script execution. |
| .github/workflows/coding-standards.yml | Updated PHP version from 8.3 to 8.4. |
| .github/workflows/static-analysis.yml | Updated PHP version from 8.3 to 8.4. |
| composer.json | Added symfony/polyfill-php83; upgraded several dev dependencies including phpunit, predis, symfony/process, and twig; reordered dev dependencies. |
| vendor-bin/tools/composer.json | Upgraded psalm to v6.12 and added rector/rector and rector/swiss-knife dependencies. |
| tests/Fake/fake-app/var/templates/Page/Html/BlogPostingDonut.html.twig | Added new Twig template rendering article and comment variables. |
| tests/Fake/FakeMobileEtagSetter.php | Updated method signature to use explicit nullable union types for parameters. |
| tests/QueryRepositoryTest.php | Modified JSON string comparison to normalize line endings for cross-platform compatibility; added helper method. |
| tests/UtiTagTest.php | Updated expected surrogate key format to reflect new underscore separator and parameter sorting. |
| src/RedisDsnProvider.php | Added support for RelayCluster type; updated return type and annotations; added #[Override] attribute. |
| src/DevEtagSetter.php | Replaced manual ETag construction with UriTag usage; added #[Override] attribute; removed unused imports. |
| src/SurrogateKeys.php | Replaced manual surrogate key construction with UriTag invocation; removed unused imports. |
| src/UriTag.php | Refined surrogate key sanitization replacing certain characters with underscores; added #[Override] attributes. |
| src/Exception/*.php (ExpireAtKeyNotExists, RedisConnectionException, ResourceStorageUnserializeException, ReturnValueIsNotResourceObjectException, UnmatchedQuery) | Marked all these exception classes as final to prevent subclassing. |
| Multiple source files under src/ (e.g., CacheInterceptor.php, CacheVersionModule.php, CommandInterceptor.php, DevEtagModule.php, DonutCacheModule.php, DonutCommandInterceptor.php, DonutRenderer.php, DonutRepository.php, DonutRequest.php, EtagSetter.php, HttpCache.php, HttpCacheInterceptor.php, MatchQuery.php, MobileEtagModule.php, MobileEtagSetter.php, NullCacheControlHeaderSetter.php, NullCdnCacheControlModule.php, NullPurger.php, ProdQueryRepositoryModule.php, QueryRepository.php, QueryRepositoryModule.php, RefreshAnnotatedCommand.php, RefreshInterceptor.php, RefreshSameCommand.php, RepositoryLogger.php, ResourceStorage.php, StorageExpiryModule.php, StorageMemcachedEtagModule.php, StorageMemcachedModule.php, StorageRedisDsnModule.php, SurrogateKeys.php, UriTag.php, etc.) | Added `#[Override]` attribute annotations to methods overriding parent or interface methods; added import statements for `Override`; no logic or control flow changes except for minor formatting in RepositoryLogger. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant GitHub Actions
participant Matrix (OS/PHP/Deps)
participant Runner
participant Composer
participant PHPUnit
participant Redis
GitHub Actions->>Matrix (OS/PHP/Deps): Start job for each combination
Matrix->>Runner: Setup environment (OS, PHP version)
Runner->>Redis: Install and start Redis and Memcached services
Runner->>Composer: Install dependencies (with/without --prefer-lowest)
Runner->>PHPUnit: Run PHPUnit tests
PHPUnit-->>Runner: Test results
Runner->>GitHub Actions: Upload code coverage (Ubuntu, PHP 8.4, highest deps only)
alt On Ubuntu & SCRIPT set
Runner->>Composer: Require doctrine/cache, update dependencies
Runner->>Runner: Run demo script
endPossibly related PRs
|
Simplified dependency resolution, updated caching, and revamped matrix strategy to support more OS combinations. Enhanced readability with consistent naming and conditional logic improvements. Updated actions versions and streamlined demo script execution.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Reviewer's GuideThe PR refactors the CI workflow by reorganizing triggers and environment variables, expanding the build matrix and services, upgrading action versions, consolidating caching, unifying dependency steps, and improving test and demo executions to enhance flexibility and performance. Flow diagram for improved dependency management and demo execution in CIflowchart TD
A[Start Job]
B[Checkout Repository]
C[Setup PHP]
D[Cache Composer Dependencies]
E[Configure Dependency Resolution]
F[Update Dependencies]
G[Run PHPUnit Tests]
H[Upload Coverage Report]
I{Is SCRIPT set and OS is ubuntu-latest?}
J[Prepare Demo Dependencies]
K[Run Demo Script]
L[End Job]
A --> B --> C --> D --> E --> F --> G --> H --> I
I -- Yes --> J --> K --> L
I -- No --> L
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @koriym - I've reviewed your changes - here's some feedback:
- The matrix exclude only removes the PHP 8.1 + lowest combo on Ubuntu—if you intended to skip 8.1/lowest on all OSes, expand the exclude or add separate rules for Windows/macOS.
- On Windows runners the Composer cache dir isn’t necessarily ~/.composer/cache; consider using a platform‐aware path or
${{ steps.setup-php.outputs.cache-dir }}to ensure caching works cross‐platform. - Using
**/composer.lockin the hashFiles key can pick up multiple lockfiles—pin it to the rootcomposer.lock(or a specific path) to avoid unintended cache misses or collisions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The matrix exclude only removes the PHP 8.1 + lowest combo on Ubuntu—if you intended to skip 8.1/lowest on all OSes, expand the exclude or add separate rules for Windows/macOS.
- On Windows runners the Composer cache dir isn’t necessarily ~/.composer/cache; consider using a platform‐aware path or `${{ steps.setup-php.outputs.cache-dir }}` to ensure caching works cross‐platform.
- Using `**/composer.lock` in the hashFiles key can pick up multiple lockfiles—pin it to the root `composer.lock` (or a specific path) to avoid unintended cache misses or collisions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/continuous-integration.yml (3)
6-6: DRY the paths-ignore patterns.
The same**/*.mdignore rule is duplicated under bothpushandpull_request. Consider combining the events to reduce duplication:on: [push, pull_request]: paths-ignore: - '**/*.md'Also applies to: 8-9
13-14: Refine environment variable scoping.
SCRIPTis only used in the demo step—move it into that step’senv:block. You can keepCOMPOSER_FLAGSat the global level since it’s consumed by multiple steps.
78-83: Optimize demo execution step.
- Scope
SCRIPTvia this step’senv:instead of global.- Running
composer updateis heavy; for demo purposes, prefer:to speed up setup.composer install --no-dev --no-interaction
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/continuous-integration.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/continuous-integration.yml
73-73: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (8)
.github/workflows/continuous-integration.yml (8)
18-19: Dynamic job naming and runner selection look good.
Naming the job with the matrix parameters and using${{ matrix.os }}forruns-onimproves clarity and consistency.
21-21: Disable fail-fast for matrix jobs.
fail-fast: falseensures every PHP/OS/dependency combination runs to completion, providing comprehensive coverage.
23-26: Verify matrix exclusions accuracy.
You’re excluding PHP 8.1 on Ubuntu and PHP 8.1 with lowest dependencies globally. Double-check that these combinations match your intended test surface (e.g., that PHP 8.1-highest still runs on macOS/Windows only).Also applies to: 29-29
43-44: Updated checkout action to v4.
Upgrading fromactions/checkout@v3to@v4modernizes the workflow and gains performance improvements.
52-52: Include Composer in PHP setup.
Addingtools: composerensures Composer is available across all runners without extra installation steps.
63-64: Clean platform config before update.
Unsetting Composer’s platform config unconditionally simplifies dependency resolution across environments.
67-67: Conditionally apply--prefer-lowest.
The expression${{ matrix.dependencies == 'lowest' && '--prefer-lowest' || '' }}correctly toggles the lowest-dependency flag. Good use of GitHub Actions expressions.
69-69: Step naming is clear.
Run PHPUnit testssuccinctly describes the action.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 1.x #161 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 232 232
===========================================
Files 51 51
Lines 739 741 +2
===========================================
+ Hits 739 741 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Revised the matrix strategy to include detailed configurations for PHP versions and dependencies across Ubuntu, Windows, and macOS. Added Redis setup steps for Windows and macOS to ensure consistency in services. Enhanced workflow flexibility and clarity.
Updated matrix strategy to better support PHP versions and dependencies across different OSes. Added Redis setup steps for all platforms, adjusted service configurations, optimized Composer caching, and streamlined conditional logic for clarity. Enhanced CI execution time and reliability.
Updated matrix strategy to include Redis service for Linux and manual setup for Windows and macOS. Adjusted Redis configuration steps and improved setup consistency across platforms.
…ti-OS behavior, update dependencies, and streamline configuration files.
…directory separator replacement
…in `SurrogateKeys` constructor
…HP 8 `int|null` typehint
…ve unused `DIRECTORY_SEPARATOR` constant
Extended CI workflow to include Memcached installation and setup for Windows and Linux runners, in addition to macOS. Updated PHP extensions to enable Memcached support.
Switch to `memcached-win64` package due to outdated `memcached` package issues. Update installation and service start steps to ensure correct functionality.
Switch to `memcached.install` package for Windows due to issues with `memcached` and non-existence of `memcached-win64`. Add steps to install and start Memcached on Ubuntu for compatibility across Linux runners. Simplify comments for better clarity.
Add installation parameter to configure Memcached as a Windows Service during setup. Update related comments and ensure proper service start with a delay for stability.
Switch from unreliable Chocolatey package to manual Memcached download and configuration. Update steps to include downloading, extracting, and running Memcached manually for improved stability and reliability.
Switch from Invoke-WebRequest to curl for more reliable downloads, add error handling for download and process checks, and enhance logging to ensure Memcached setup stability.
Switch to dedicated GitHub action for downloading Memcached releases, removing unreliable curl-based downloads. Simplify extraction and setup steps, enhance logging, and refine process validation to improve reliability.
…erride warning in RedisDsnProvider.
…itization to replace only forward slashes.
…t`, `predis/predis`, `twig/twig`, `symfony/process`)
…es. Update `vimeo/psalm` to version 6.12.
…kflows. Remove redundant comment from CI workflow.
… sanitization logic and improving URI handling.
…bin/tools/composer.json`
…cross parameter order.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
koriym
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/continuous-integration.yml (1)
161-167: Cache configuration needs cross-platform improvement.The hardcoded
~/.composer/cachepath won't work correctly on Windows. Consider using a cross-platform approach.Use the Composer cache directory environment variable for better cross-platform compatibility:
- path: ~/.composer/cache + path: ${{ env.COMPOSER_CACHE_DIR }}Alternatively, set
COMPOSER_CACHE_DIRin the environment variables or include Windows-specific paths.
🧹 Nitpick comments (1)
.github/workflows/continuous-integration.yml (1)
127-127: Fix trailing whitespace.Remove trailing spaces as flagged by YAMLlint.
- # The URL was found and verified by the user. + # The URL was found and verified by the user.- # Running it as a simple process is the most robust and correct approach here. + # Running it as a simple process is the most robust and correct approach here.Also applies to: 135-135
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
vendor-bin/tools/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (60)
.github/workflows/coding-standards.yml(1 hunks).github/workflows/continuous-integration.yml(1 hunks).github/workflows/static-analysis.yml(1 hunks)composer.json(1 hunks)src/AbstractDonutCacheInterceptor.php(2 hunks)src/CacheDependency.php(2 hunks)src/CacheInterceptor.php(2 hunks)src/CacheVersionModule.php(2 hunks)src/CacheableModule.php(2 hunks)src/Cdn/AkamaiCacheControlHeaderSetter.php(2 hunks)src/Cdn/AkamaiModule.php(2 hunks)src/Cdn/FastlyCacheControlHeaderSetter.php(1 hunks)src/Cdn/FastlyCachePurger.php(1 hunks)src/Cdn/FastlyModule.php(2 hunks)src/CdnCacheControlHeaderSetter.php(2 hunks)src/CliHttpCache.php(3 hunks)src/CommandInterceptor.php(2 hunks)src/CommandsProvider.php(2 hunks)src/DevEtagModule.php(2 hunks)src/DevEtagSetter.php(2 hunks)src/DonutCacheModule.php(2 hunks)src/DonutCommandInterceptor.php(1 hunks)src/DonutRenderer.php(2 hunks)src/DonutRepository.php(6 hunks)src/DonutRequest.php(2 hunks)src/EtagSetter.php(2 hunks)src/Exception/ExpireAtKeyNotExists.php(1 hunks)src/Exception/RedisConnectionException.php(1 hunks)src/Exception/ResourceStorageUnserializeException.php(1 hunks)src/Exception/ReturnValueIsNotResourceObjectException.php(1 hunks)src/Exception/UnmatchedQuery.php(1 hunks)src/HttpCache.php(3 hunks)src/HttpCacheInterceptor.php(2 hunks)src/MatchQuery.php(2 hunks)src/MobileEtagModule.php(2 hunks)src/MobileEtagSetter.php(2 hunks)src/NullCacheControlHeaderSetter.php(1 hunks)src/NullCdnCacheControlModule.php(1 hunks)src/NullPurger.php(1 hunks)src/ProdQueryRepositoryModule.php(2 hunks)src/QueryRepository.php(4 hunks)src/QueryRepositoryModule.php(2 hunks)src/RedisDsnProvider.php(2 hunks)src/RefreshAnnotatedCommand.php(2 hunks)src/RefreshInterceptor.php(2 hunks)src/RefreshSameCommand.php(1 hunks)src/RepositoryLogger.php(3 hunks)src/ResourceStorage.php(11 hunks)src/StorageExpiryModule.php(2 hunks)src/StorageMemcachedEtagModule.php(2 hunks)src/StorageMemcachedModule.php(2 hunks)src/StorageRedisDsnModule.php(2 hunks)src/SurrogateKeys.php(1 hunks)src/UriTag.php(2 hunks)tests/Fake/FakeMobileEtagSetter.php(1 hunks)tests/Fake/fake-app/var/templates/Page/Html/BlogPostingDonut.html.twig(0 hunks)tests/Fake/fake-app/var/templates/Page/Html/BlogPostingDonut.html.twig(1 hunks)tests/QueryRepositoryTest.php(2 hunks)tests/UtiTagTest.php(1 hunks)vendor-bin/tools/composer.json(1 hunks)
✅ Files skipped from review due to trivial changes (41)
- src/CacheableModule.php
- src/MatchQuery.php
- src/CacheInterceptor.php
- src/MobileEtagModule.php
- src/CdnCacheControlHeaderSetter.php
- tests/Fake/fake-app/var/templates/Page/Html/BlogPostingDonut.html.twig
- src/Cdn/AkamaiCacheControlHeaderSetter.php
- src/EtagSetter.php
- src/CacheDependency.php
- src/MobileEtagSetter.php
- src/Exception/RedisConnectionException.php
- src/Cdn/AkamaiModule.php
- src/DonutRenderer.php
- src/Exception/ResourceStorageUnserializeException.php
- .github/workflows/coding-standards.yml
- .github/workflows/static-analysis.yml
- src/DonutRequest.php
- src/DonutCommandInterceptor.php
- src/Exception/UnmatchedQuery.php
- src/CommandInterceptor.php
- src/NullCdnCacheControlModule.php
- src/StorageRedisDsnModule.php
- src/StorageExpiryModule.php
- src/CliHttpCache.php
- src/RefreshSameCommand.php
- src/QueryRepositoryModule.php
- src/HttpCacheInterceptor.php
- src/Exception/ReturnValueIsNotResourceObjectException.php
- src/DonutCacheModule.php
- src/AbstractDonutCacheInterceptor.php
- src/Cdn/FastlyCacheControlHeaderSetter.php
- src/Cdn/FastlyModule.php
- src/Exception/ExpireAtKeyNotExists.php
- src/ProdQueryRepositoryModule.php
- src/StorageMemcachedEtagModule.php
- src/Cdn/FastlyCachePurger.php
- tests/Fake/fake-app/var/templates/Page/Html/BlogPostingDonut.html.twig
- src/HttpCache.php
- src/CommandsProvider.php
- composer.json
- src/DonutRepository.php
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/continuous-integration.yml
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 135-135: trailing spaces
(trailing-spaces)
🔇 Additional comments (35)
tests/Fake/FakeMobileEtagSetter.php (1)
22-22: LGTM! Improved type safety with explicit union types.The change from implicit nullable parameters to explicit union types (
int|null,HttpCache|null) enhances code clarity and follows modern PHP best practices. This makes the method signature more explicit about accepting null values.src/NullPurger.php (1)
7-7: LGTM! Excellent use of the Override attribute.The addition of the
#[Override]attribute provides explicit documentation that this method overrides thePurgerInterface::__invoke()method and enables compile-time verification of the override relationship.Also applies to: 11-11
src/DevEtagModule.php (1)
7-7: LGTM! Proper override annotation added.The
#[Override]attribute correctly documents that this method overridesAbstractModule::configure()and provides compile-time safety by ensuring the override relationship is valid.Also applies to: 22-22
src/RefreshInterceptor.php (1)
10-10: LGTM! Override attribute correctly applied.The
#[Override]attribute appropriately documents that this method implementsMethodInterceptor::invoke()and provides valuable compile-time verification of the interface contract.Also applies to: 21-21
src/CacheVersionModule.php (1)
7-7: LGTM! Override attribute enhances method documentation.The
#[Override]attribute correctly indicates that this method overridesAbstractModule::configure()and enables compile-time validation of the override relationship.Also applies to: 30-30
src/RefreshAnnotatedCommand.php (1)
13-13: LGTM! Override attribute properly documents interface implementation.The
#[Override]attribute correctly marks this method as implementingCommandInterface::command()and provides compile-time assurance that the interface contract is properly fulfilled.Also applies to: 27-27
src/NullCacheControlHeaderSetter.php (1)
8-8: Excellent addition of explicit override annotation.The
#[Override]attribute clearly documents that this method implements the interface contract, improving code clarity and enabling better IDE/tooling support.Also applies to: 12-12
src/RepositoryLogger.php (2)
7-7: Good addition of explicit override annotations.The
#[Override]attributes properly document interface method implementations, enhancing code clarity and tooling support.Also applies to: 12-12, 24-24, 41-41
36-36: Functionally equivalent sprintf to vsprintf conversion.The change from
sprintf(...$values)tovsprintf($template, $values)maintains identical functionality while using a different argument passing approach. Both implementations produce the same result.src/QueryRepository.php (1)
13-13: Excellent systematic addition of override annotations.All three public methods correctly implement
QueryRepositoryInterface, and the#[Override]attributes clearly document these interface implementations. This improves code maintainability and enables better IDE support.Also applies to: 35-35, 61-61, 78-78
tests/QueryRepositoryTest.php (2)
33-33: Good addition of str_replace import for line ending normalization.The import supports the cross-platform compatibility improvements in the test.
137-140: Excellent cross-platform line ending normalization helper.The
normalizeLineEndingsmethod properly handles Windows (\r\n) and old Mac (\r) line endings, converting them to Unix (\n) style. This aligns perfectly with the PR's multi-OS CI testing objectives.src/ResourceStorage.php (2)
12-12: Comprehensive and systematic override annotations.All public methods correctly implement
ResourceStorageInterface, and the#[Override]attributes systematically document these implementations. This significantly improves code maintainability and enables better IDE/tooling support across the entire storage interface.Also applies to: 90-90, 100-100, 114-114, 123-123, 134-134, 151-151, 169-169, 185-185, 194-194, 260-260
268-268: Helpful documentation comment for etag sanitization.The comment clarifies that the etag parameter is sanitized before saving, providing useful context for understanding the storage behavior.
src/StorageMemcachedModule.php (1)
9-9: LGTM! Good use of #[Override] attribute for code clarity.The addition of the
#[Override]attribute improves code documentation by explicitly marking theconfigure()method as overriding the parentAbstractModule::configure()method. This provides better IDE support and helps catch signature mismatches.Also applies to: 31-31
vendor-bin/tools/composer.json (1)
6-8: Verify compatibility of major version upgrades.The addition of Rector tools is excellent for code quality automation, and the Psalm upgrade from v5 to v6 brings improved static analysis. However, major version upgrades may introduce breaking changes or new analysis rules.
Please test the updated tools to ensure they work correctly with the current codebase:
#!/bin/bash # Description: Test the updated static analysis tools # Expected: All tools should run without errors and be compatible with PHP 8.1+ # Test Rector echo "Testing Rector..." vendor-bin/tools/vendor/bin/rector --version vendor-bin/tools/vendor/bin/rector process --dry-run src/ || echo "Rector failed" # Test Psalm echo "Testing Psalm..." vendor-bin/tools/vendor/bin/psalm --version vendor-bin/tools/vendor/bin/psalm --show-info=false || echo "Psalm failed" # Test swiss-knife echo "Testing Swiss Knife..." vendor-bin/tools/vendor/bin/swiss-knife --version || echo "Swiss Knife failed"tests/UtiTagTest.php (1)
15-17: Excellent test coverage for UriTag normalization.The updated test correctly validates the new UriTag behavior:
- Query parameters are now separated by underscores (more cache-friendly)
- Parameter order normalization ensures consistent cache keys regardless of query parameter order
This improves cache efficiency by ensuring
?a=1&b=2and?b=2&a=1produce identical cache keys.src/SurrogateKeys.php (1)
25-25: Good refactoring to centralize URI key generation.Delegating surrogate key generation to
UriTagimproves code maintainability by:
- Centralizing URI-to-key conversion logic
- Eliminating duplicate string manipulation code
- Ensuring consistent key generation across the codebase
This aligns well with the UriTag normalization changes validated in the tests.
src/DevEtagSetter.php (2)
11-11: Good addition of #[Override] attribute.The
#[Override]attribute properly documents that this method implements theEtagSetterInterface::__invoke()method, improving code clarity and IDE support.Also applies to: 25-25
28-32: Excellent refactoring to use UriTag for consistency.The refactoring to use
UriTagfor ETag generation:
- Ensures consistent URI-to-key conversion across the codebase
- Simplifies the code by removing manual string manipulation
- Provides helpful comments explaining the dev-mode behavior
- Aligns with the UriTag normalization improvements
This creates a unified approach to URI-based cache key generation throughout the system.
src/UriTag.php (4)
9-9: Good practice: Adding Override import.The addition of the
Overrideimport enables explicit marking of overriding methods, improving code clarity and enabling static analysis benefits.
23-23: Correct use of Override attribute.The
#[Override]attribute correctly marks this method as overriding the interface method, improving code documentation and enabling compile-time validation.
29-33: Excellent URI sanitization improvement.The refined logic now properly sanitizes URI keys by replacing
/,?, and&with underscores, making them compatible with Symfony cache adapter requirements. The comment clearly explains the purpose and benefits.
39-39: Consistent Override attribute usage.Good consistency in marking interface method overrides with the
#[Override]attribute.src/RedisDsnProvider.php (4)
9-9: Good addition of Override import.Consistent with the broader codebase pattern of adding explicit Override attributes.
15-15: Proper import alias for RelayCluster.The import alias
Relay\Cluster as RelayClusterclearly differentiates from other cluster types and maintains readability.
19-19: Comprehensive type annotation update.The generic type annotation correctly includes
RelayClusteralongside existing Redis connection types, maintaining type safety.
34-41: Enhanced type safety and documentation.The
#[Override]attribute and typed variable assignment improve both documentation and type safety. The explicit typing of the$connectionvariable ensures proper type validation at runtime..github/workflows/continuous-integration.yml (7)
6-13: Good optimization: Ignoring documentation-only changes.The path ignores for markdown files, text files, and LICENSE prevent unnecessary CI runs for documentation-only changes, improving efficiency.
22-64: Excellent multi-platform testing matrix.The structured approach to testing across Ubuntu (all PHP versions), Windows and macOS (latest PHP only) provides comprehensive coverage while balancing CI resource usage. The clear commenting makes the strategy easy to understand.
67-68: Good: Updated checkout action to v4.Upgrading from v3 to v4 ensures compatibility with the latest GitHub Actions infrastructure.
71-97: Comprehensive Redis setup across platforms.The platform-specific Redis installation and startup logic properly handles the differences between Linux, Windows, and macOS environments. Good use of conditional steps and verification with
redis-cli ping.
112-149: Robust Windows Memcached implementation.The detailed Windows Memcached setup addresses the complexity of installing Memcached on Windows with proper error handling and verification steps. The comments clearly explain the approach and reasoning.
179-179: Codecov action version updated correctly.Good update from v3 to v4, addressing the previous review feedback about the outdated action version.
184-189: Well-implemented conditional demo script execution.The demo script step is properly conditioned to run only on Ubuntu with PHP 8.4 and highest dependencies, with appropriate dependency management before execution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Refactored the CI workflow to enhance maintainability, support multiple OS and PHP versions, adjust matrix configurations, streamline dependency management, introduce environment-based conditions, update step names, and improve Composer caching for better performance.
Summary by Sourcery
Refactor the CI workflow to improve flexibility and efficiency by broadening OS/PHP coverage, streamlining dependency management, upgrading core actions, and enhancing caching strategies.
CI: