-
-
Notifications
You must be signed in to change notification settings - Fork 27
Maintenance #145
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
Maintenance #145
Conversation
WalkthroughThis update refines project configuration and documentation. It improves code style and test scripts, enhances the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant ExternalWorkflow
Developer->>GitHub: Pushes code to branch
GitHub->>GitHub Actions: Triggers cs-fix workflow
GitHub Actions->>ExternalWorkflow: Calls spiral/gh-actions cs-fix workflow
ExternalWorkflow-->>GitHub Actions: Performs code style fix
GitHub Actions-->>Developer: Reports workflow results
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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
🧹 Nitpick comments (7)
.github/workflows/cs-fix.yml (2)
12-12
: Pin reusable workflow to a fixed version
Referencingspiral/gh-actions/...@master
can introduce breaking changes when upstream updates. It’s safer to pin to a released tag or commit SHA (e.g.,@v1.2.0
).
1-4
: Consider adding a pull_request trigger or limiting branches
Currently, the workflow runs on every push to any branch. To catch style issues early on contributions, consider adding:on: push: branches: [ main ] pull_request: branches: [ main ]composer.json (1)
83-87
: Harmonize composer script naming
We now mix hyphens and colons in script names (test:unit
vstest-cover
). For consistency, consider renaming"test-cover"
to"test:cover"
(or vice versa) and update invocations in CI/Makefile accordingly.README.md (4)
15-21
: Vary bullet phrasing to avoid repetition
Three consecutive list items start with “You want to…”. Consider rewording for readability, e.g.:- You want to use Temporal. + Leverage Temporal for orchestrating workflows.🧰 Tools
🪛 LanguageTool
[style] ~19-~19: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ins like gRPC, Queue, KV, and others. - You want to use Temporal. - You want to con...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~19-~19: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...like gRPC, Queue, KV, and others. - You want to use Temporal. - You want to control Roa...(REP_WANT_TO_VB)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d others. - You want to use Temporal. - You want to control RoadRunner configuratio...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~20-~20: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...hers. - You want to use Temporal. - You want to control RoadRunner configuration. ![Ro...(REP_WANT_TO_VB)
[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ... to use Temporal. - You want to control RoadRunner configuration. 
59-63
: Add missing article for clarity
The line reads “Download and install RoadRunner binary using DLoad:”. It’s clearer as:- Download and install RoadRunner binary using DLoad: + Download and install the RoadRunner binary using DLoad:🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... --tag=configDownload and install RoadRunner binary using DLoad:
shell ./vendor/...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
92-98
: Clarify the RR binary path
If installed via DLoad intovendor/bin
, running./rr serve
may not work without a PATH update. Either instruct users to run:./vendor/bin/rr serve
or note that
vendor/bin
should be in the PATH.
139-141
: Add a TIP admonition for gRPC plugin
HTTP and Jobs sections use[!TIP]
blocks. Consider adding one for gRPC:> [!TIP] > Read more about the gRPC plugin in the RoadRunner docs: [docs](https://docs.roadrunner.dev/docs/grpc)This keeps the style consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.editorconfig
(1 hunks).gitattributes
(1 hunks).github/workflows/cs-fix.yml
(1 hunks).php-cs-fixer.dist.php
(1 hunks)Makefile
(1 hunks)README.md
(9 hunks)composer.json
(1 hunks)phpunit.xml.dist
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~19-~19: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ins like gRPC, Queue, KV, and others. - You want to use Temporal. - You want to con...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~19-~19: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...like gRPC, Queue, KV, and others. - You want to use Temporal. - You want to control Roa...
(REP_WANT_TO_VB)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d others. - You want to use Temporal. - You want to control RoadRunner configuratio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~20-~20: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...hers. - You want to use Temporal. - You want to control RoadRunner configuration. ![Ro...
(REP_WANT_TO_VB)
[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ... to use Temporal. - You want to control RoadRunner configuration. 
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... --tag=config Download and install RoadRunner binary using DLoad:
shell ./vendor/...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (11)
.php-cs-fixer.dist.php (1)
13-13
: Good explicit setting for risky fixersSetting
allowRisky(false)
explicitly makes it clear that risky fixers (those that might change code behavior) should not be applied. This is a good practice for code style enforcement in CI..gitattributes (3)
3-3
: Improved exclusion pattern for hidden filesThe pattern now properly ignores all dotfiles and dot directories when exporting the repository, which is more comprehensive than the previous approach.
6-7
: Good explicit exclusion of XML configuration filesAdding specific patterns for XML files in the root directory helps ensure configuration files aren't included in distribution archives.
10-10
: Appropriate exclusion of rector.phpAdding rector.php to the export-ignore list ensures this development tool configuration isn't included in distribution packages.
phpunit.xml.dist (4)
5-9
: Enhanced PHPUnit configuration attributesThe added attributes improve test reporting and debugging:
cacheResultFile
specifies a standardized location for caching resultsstderr="true"
ensures errors are properly displayed- The display options for warnings and deprecations improve visibility of potential issues
These changes follow modern PHPUnit best practices.
19-26
: Improved source coverage configurationRestructuring the coverage configuration with the
<source>
element makes the included and excluded directories more explicit, which follows the current PHPUnit recommended structure.
27-33
: Standardized coverage report pathsDirecting all coverage reports to the
runtime
directory creates a more organized structure and keeps the project root clean.
34-36
: Added JUnit XML reportingAdding JUnit XML output improves CI integration capabilities and provides a standardized format for test results that many tools can consume.
.editorconfig (1)
11-11
: Fixed EditorConfig syntaxRemoved unnecessary spaces after commas in the file pattern list, which ensures proper parsing of the EditorConfig rules for the specified file types.
composer.json (2)
23-34
: Great—authors list updated
Addingroxblnfk
andtarampampam
improves attribution. Ensure this list stays up to date with active maintainers.
99-102
: Nice addition of support metadata
Including"support"
links for issues and source enhances discoverability for contributors and users.
test-cover: ## Execute php tests with coverage | ||
docker-compose run --rm --user "0:0" -e 'XDEBUG_MODE=coverage' app sh -c 'docker-php-ext-enable xdebug && su $(shell whoami) -s /bin/sh -c "composer phpunit-cover"' | ||
docker-compose run --rm --user "0:0" -e 'XDEBUG_MODE=coverage' app sh -c 'docker-php-ext-enable xdebug && su $(shell whoami) -s /bin/sh -c "composer phpunit:cover"' |
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.
💡 Verification agent
🧩 Analysis chain
Mismatch in composer script invocation for test-cover
The Makefile’s test-cover
target calls composer phpunit:cover
, but in composer.json
the scripts are defined as "phpunit-cover"
and "test-cover"
. Invoking composer phpunit:cover
will fail.
- If you intend to run the composite coverage script, update to
composer test-cover
. - If you meant the standalone coverage script, use
composer phpunit-cover
.
🏁 Script executed:
#!/bin/bash
# Check composer.json for script definitions matching phpunit-cover, test-cover, or phpunit:cover
grep -n '"scripts"' -n composer.json
grep -n '"phpunit-cover"' composer.json
grep -n '"test-cover"' composer.json
grep -n '"phpunit:cover"' composer.json
Length of output: 247
Incorrect Makefile composer script invocation
The test-cover
target in Makefile
(lines 31–32) calls:
su $(shell whoami) -s /bin/sh -c "composer phpunit:cover"
However, your composer.json
defines its scripts with dashes, not colons:
• "phpunit-cover"
(standalone coverage)
• "test-cover"
(composite coverage)
Recommended fixes:
- If you want the composite coverage flow, change to
… su $(shell whoami) -s /bin/sh -c "composer test-cover"
- If you just intended the PHPUnit coverage step, use
… su $(shell whoami) -s /bin/sh -c "composer phpunit-cover"
Please update the invocation accordingly.
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (3)
README.md (3)
15-19
: Clean up blank line and standardize bullet punctuation
There’s an extra blank line at 16 and the bullets in 17–19 are inconsistent in punctuation. Recommend removing the empty line and either adding periods at the end of each item or dropping them all.@@ -15,5 +15,4 - This package provides complete Laravel integration with RoadRunner, offering: - - - Support for HTTP and other RoadRunner plugins (gRPC, Queue, KV) - - [Temporal](https://temporal.io/) integration - - Full RoadRunner configuration control + This package provides complete Laravel integration with RoadRunner, offering: + - Support for HTTP and other RoadRunner plugins (gRPC, Queue, KV). + - [Temporal](https://temporal.io/) integration. + - Full RoadRunner configuration control.
101-114
: Fix punctuation and casing in “How It Works” section
Lines 113–114 end with a comma but should use a period, and ensure “Octane” is consistently capitalized.@@ -113,2 +113,2 - and handles them using the [Octane][octane] worker, + and handles them using the [Octane][octane] worker.
143-144
: Merge broken sentence for readability
The two-line paragraph should be combined into one for clarity and to avoid line-break issues in Markdown.- The Queue plugin allows you to use RoadRunner as a queue driver for Laravel - without additional services like Redis or a database. + The Queue plugin allows you to use RoadRunner as a queue driver for Laravel without additional services like Redis or a database.
🛑 Comments failed to post (1)
README.md (1)
91-97: 🛠️ Refactor suggestion
Use the correct path for the RoadRunner binary
Since the binary is installed via DLoad in thevendor/bin
directory, update the command to:- ### Starting the Server - - Start the RoadRunner server with: - ```shell - ./rr serve - ``` + ### Starting the Server + + Start the RoadRunner server with: + ```shell + ./vendor/bin/rr serve + ```
Description
Add CS fixer in CI
Update metafiles
Update readme
Summary by CodeRabbit