Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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. 📝 WalkthroughWalkthroughA docblock comment was added to the ChangesDocumentation Addition
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/StringCase.php`:
- Around line 16-19: The docblock claiming StringCase is unused conflicts with
the hydration contract; update code or docs so they match: either implement
snake_case→camelCase conversion using the StringCase utility in the hydration
paths (locations using FetchClass / FetchNewInstance and PDO::FETCH_CLASS /
PDO::FETCH_FUNC, and any hydrate() implementations) so StringCase (and the old
CamelCaseTrait) is actually applied during entity hydration, or modify this
docblock to remove the claim that StringCase is unused and explicitly document
that hydration relies on StringCase per the coding guideline; ensure references
to StringCase, CamelCaseTrait, FetchClass, FetchNewInstance, and the hydrate
flow are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| * 1.0.0-rc1 で `CamelCaseTrait` の代替として追加されたが、現在の hydrate 経路 | ||
| * （`FetchClass` の `PDO::FETCH_CLASS` と `FetchNewInstance` の `PDO::FETCH_FUNC`） | ||
| * はいずれも列名→プロパティ名の変換を行わないため、ライブラリ内部からは利用されていない。 | ||
| * 公開APIとして残しているだけで、将来のメジャーリリースで削除する可能性がある。 |
There was a problem hiding this comment.
Documentation conflicts with the repository hydration contract.
This docblock states hydration does not perform snake_case→camelCase conversion and that StringCase is effectively dead, which contradicts the project rule that hydration should use StringCase conversion. Please reconcile this by either implementing the documented guideline in hydration paths or updating the guideline/spec before merging this statement.
As per coding guidelines, "Use snake_case to camelCase conversion via StringCase utility for entity hydration".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/StringCase.php` around lines 16 - 19, The docblock claiming StringCase is
unused conflicts with the hydration contract; update code or docs so they match:
either implement snake_case→camelCase conversion using the StringCase utility in
the hydration paths (locations using FetchClass / FetchNewInstance and
PDO::FETCH_CLASS / PDO::FETCH_FUNC, and any hydrate() implementations) so
StringCase (and the old CamelCaseTrait) is actually applied during entity
hydration, or modify this docblock to remove the claim that StringCase is unused
and explicitly document that hydration relies on StringCase per the coding
guideline; ensure references to StringCase, CamelCaseTrait, FetchClass,
FetchNewInstance, and the hydrate flow are consistent.
koriym
force-pushed
the
deprecate-string-case
branch
2 times, most recently
from
2865d52 to
465765a
Compare
StringCase was added in 1.0.0-rc1 as a replacement for the removed CamelCaseTrait, but neither hydration path uses it. FetchClass relies on PDO::FETCH_CLASS (column name = property name; no conversion) and FetchNewInstance uses PDO::FETCH_FUNC with positional binding (column name irrelevant). The class is kept only for backward compatibility.
koriym
force-pushed
the
deprecate-string-case
branch
from
465765a to
9307894
Compare
1 participant
Summary
StringCasewas added in 1.0.0-rc1 as a replacement for the removedCamelCaseTrait, but neither hydration path actually uses it:FetchClassusesPDO::FETCH_CLASS(column name = property name; no conversion)FetchNewInstanceusesPDO::FETCH_FUNCwith positional binding (column name irrelevant)grep StringCase:: src/returns zero hits across the entire git history ofsrc/. The class has effectively been dead code since it shipped.This PR adds a class-level PHPDoc note explaining the situation and flagging the class as a candidate for removal in a future major release. No behavioral change.
Test plan
./vendor/bin/phpunit --no-coverage tests/StringCaseTest.phppasses./vendor/bin/phpcs --standard=PSR12 src/StringCase.phppassesRelated
Surfaced while reviewing the Ray.MediaQuery 1.0 docs (bearsunday/bearsunday.github.io#348), which incorrectly described an "automatic snake_case → camelCase conversion" that does not exist.
Summary by CodeRabbit