Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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?
stof commentedMay 2, 2016
Can the caster be tested ? |
nicolas-grekas commentedMay 2, 2016
Tests added, comments addressed. |
javiereguiluz commentedMay 4, 2016
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 to0b73d77Comparenicolas-grekas commentedMay 19, 2016
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 connectionsCommits-------56ae8c8 [VarDumper] Add Redis caster
Helps working on Redis connections