Conversation
…#1283)
- go_cli_helpers.go: drop backtick-quoted `fmt`/`strings` imports
- phpFailure/phpWrapMessage/phpWrapAction now use core.E + core.Sprintf
instead of fmt.Errorf
- strings.HasPrefix/TrimPrefix → core.HasPrefix/TrimPrefix
- strings.Cut(body, "=") → phpCutOnEquals helper using core.SplitN
- strings.ToLower → core.Lower
- 27 callers using `phpFailure(cliWrapErrorFormat, msg, err)` now use
`core.E("php", msg, err)` directly (cleaner — no format-string indirection
for the wrapped-error path)
- 4 remaining direct phpFailure("...%w...", err) sites converted to core.E
with explicit cause (core.Sprintf doesn't support %w)
Verification:
- GOWORK=off go build ./... clean
- GOWORK=off go test -count=1 -short ./pkg/php/ pass
Closes tasks.lthn.sh/view.php?id=1283
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PHP package's error handling is being standardised across all command handlers and utility functions. The old Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 0/5 reviews remaining, refill in 36 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
go/pkg/php/cmd_dev.go (1)
43-47: 💤 Low valueConsider using i18n key for consistency.
This error uses a hardcoded English string, whereas similar
os.Getwd()failures in other files (e.g.,cmd_ci.goline 90) usephpT(i18nFailGetKey, workingDirectorySubject).♻️ Optional: Use i18n pattern
cwd, err := os.Getwd() if err != nil { - return core.E("php", "failed to get working directory", err) + return core.E("php", phpT(i18nFailGetKey, workingDirectorySubject), err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/pkg/php/cmd_dev.go` around lines 43 - 47, The error message in runPHPDev uses a hardcoded English string; replace it to use the i18n helper so messages are consistent by calling phpT(i18nFailGetKey, workingDirectorySubject) instead of the literal "failed to get working directory" when returning core.E in runPHPDev.go/pkg/php/cmd.go (1)
118-120: 💤 Low valueConsider using i18n key for consistency.
The error message uses a hardcoded English string, whereas other files in this PR use the
phpT(i18nFailGetKey, ...)pattern for similar directory-related errors. This is a minor inconsistency that could be aligned for localisation support.♻️ Optional: Use i18n pattern
if err := os.Chdir(targetDir); err != nil { - return core.E("php", "failed to change directory to active package", err) + return core.E("php", phpT(i18nFailGetKey, "active package directory"), err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/pkg/php/cmd.go` around lines 118 - 120, Replace the hardcoded error string in the os.Chdir error handling with the i18n helper used elsewhere: call phpT(i18nFailGetKey, "failed to change directory to active package", targetDir) (or the appropriate phpT invocation pattern used across the PR) and pass that resulting message into core.E("php", ..., err) instead of the literal string; update the os.Chdir error branch (the block that currently calls core.E("php", "failed to change directory to active package", err)) to use phpT and include targetDir for context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@go/pkg/php/cmd_dev.go`:
- Around line 43-47: The error message in runPHPDev uses a hardcoded English
string; replace it to use the i18n helper so messages are consistent by calling
phpT(i18nFailGetKey, workingDirectorySubject) instead of the literal "failed to
get working directory" when returning core.E in runPHPDev.
In `@go/pkg/php/cmd.go`:
- Around line 118-120: Replace the hardcoded error string in the os.Chdir error
handling with the i18n helper used elsewhere: call phpT(i18nFailGetKey, "failed
to change directory to active package", targetDir) (or the appropriate phpT
invocation pattern used across the PR) and pass that resulting message into
core.E("php", ..., err) instead of the literal string; update the os.Chdir error
branch (the block that currently calls core.E("php", "failed to change directory
to active package", err)) to use phpT and include targetDir for context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43ffb6a2-af51-4ca4-879e-cbcdc113459e
📒 Files selected for processing (8)
go/pkg/php/cmd.gogo/pkg/php/cmd_build.gogo/pkg/php/cmd_ci.gogo/pkg/php/cmd_deploy.gogo/pkg/php/cmd_dev.gogo/pkg/php/cmd_packages.gogo/pkg/php/go_cli_helpers.gogo/pkg/php/packages.go
Added: pint.json .github/workflows/ci.yml
|
1 participant
…#1283)
fmt/stringsimportsphpFailure(cliWrapErrorFormat, msg, err)now usecore.E("php", msg, err)directly (cleaner — no format-string indirection for the wrapped-error path)Verification:
Closes tasks.lthn.sh/view.php?id=1283
Pull Request
Description
Please provide a clear description of your changes and the motivation behind them.
Fixes # (issue)
Type of Change
Testing
Please describe the tests you ran to verify your changes:
Test Configuration:
Checklist
Screenshots (if applicable)
Add screenshots to help explain your changes.
Breaking Changes
If this PR introduces breaking changes, please describe:
Additional Notes
Add any other context about the pull request here.
For Maintainers:
Summary by CodeRabbit