-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[VarDumper] Add Redis caster #18675
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
[VarDumper] Add Redis caster #18675
Conversation
*/ | ||
class RedisCaster | ||
{ | ||
private static $serializer = array( |
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.
Are you planning to add the docblocks when the PR will be ready to merge?
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.
Nope, I'm not planning any doc block here...
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.
@nicolas-grekas, that makes me wonder: which are the criteria we use to decide what to document and what to not document?
Can the caster be tested ? |
Tests added, comments addressed. |
This feature looks great ... but the code sometimes is not very readable because of the single-letter variable names ( |
$redis = new \Redis(); | ||
|
||
if (defined('HHVM_VERSION_ID')) { | ||
$xCast = <<<EODUMP |
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.
Should we use nowdoc? See #17086.
e79b572
to
0b73d77
Compare
0b73d77
to
56ae8c8
Compare
ping @symfony/deciders |
This PR was merged into the 3.2-dev branch. Discussion ---------- [VarDumper] Add Redis caster | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Helps working on Redis connections Commits ------- 56ae8c8 [VarDumper] Add Redis caster
Helps working on Redis connections