Add --custom-function parameter to check #6
Conversation
…f functions to be checked
There was a problem hiding this comment.
Perfect! Thank you for your PR. It looks great.
Here are some comments for improve code and repository history.
- Can you help with squash commits? We can call together (like whereby.com) and I can show you how do it.
- Can you rebase it to current master?
- We can add changelog line, it is perfect new feature.
src/Settings.php
Outdated
| @@ -138,7 +138,7 @@ public static function parseArguments(array $arguments) | |||
| $setting->functionsToCheck = array_unique($setting->functionsToCheck); | |||
|
|
|||
| // Any custom function is given | |||
| if (count($customFunctions) > 0) { | |||
| if (!empty($customFunctions)) { | |||
| @@ -48,7 +48,7 @@ Options for run | |||
| - `--doctrine` - check dump: `Doctrine::dump`, `\Doctrine\Common\Util\Debug::dump` | |||
| - `--symfony` - check dump: `dump`, `VarDumper::dump`, `VarDumper::setHandler` | |||
| - `--laravel` - check dump: `dd`, `dump` | |||
| - `--custom-function` - comma separated custom function name(s) to check like "pre_echo" | |||
| - `--custom-function` - comma separated custom function name(s) to check like `pre_echo` | |||
There was a problem hiding this comment.
This commit can be squash with implementation
.gitignore
Outdated
| @@ -1,3 +1,2 @@ | |||
| /vendor/ | |||
| /composer.lock | |||
| .idea | |||
There was a problem hiding this comment.
This commit can be squashed with implementing commit
| @@ -0,0 +1,65 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Please, why isn't this commit included in implementation? I mean that will be better for people who read Git history :)
|
|
||
| class CustomFunctionTest extends PHPUnit_Framework_TestCase | ||
| { | ||
| public function __construct() |
There was a problem hiding this comment.
Empty construction we can remove, what do you mean?
|
|
||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
What about add test for none function founded? (I mean when code does not includes functionName1) It coverage all cases
src/Settings.php
Outdated
| @@ -70,8 +70,9 @@ class Settings | |||
| */ | |||
| public static function parseArguments(array $arguments) | |||
| { | |||
| $arguments = new ArrayIterator(array_slice($arguments, 1)); | |||
| $setting = new self; | |||
| $arguments = new ArrayIterator(array_slice($arguments, 1)); | |||
There was a problem hiding this comment.
Please do not horizontal align code, it is not used more in the codebase
|
Hi @grogy , Many thanks for the review and feedbacks. This was a very old PR coming from the old repo :). I will fix the problems and make the PR ready for merge asap. |
Caused by the review message in the following link. #6 (comment)
Using the caret is the recommended manner of setting a version requirement.
Array access using curly braces has been deprecated in PHP 7.4. Ref: https://www.php.net/manual/en/migration74.deprecated.php#migration74.deprecated.core.array-string-access-curly-brace
Caused by the review message in the following link. #6 (comment)
|
I think I need to close this PR and come with a clean PR. |
This pull request is the clone of the pull request for the previous repository (JakubOnderka/PHP-Var-Dump-Check).
Closes #5