Code Improvements #23
Conversation
- DRY ( Don't repeat yourself concept )
- DRY ( Don't repeat yourself concept )
- Avoid updating the application version in future releases
src/helpers.php
Outdated
| * | ||
| * @return void | ||
| */ | ||
| function mkdir_if_not_exist($dir, $mode = 0700, $recursive = true) |
There was a problem hiding this comment.
since this function is only relevant for creating hooks, I would prefer something like create_hooks_dir. That way, we would only need to pass in $gitDir at the call sites. We can all just remove the other arguments and hardcode in the mkdir call.
src/helpers.php
Outdated
| * | ||
| * @return bool | ||
| */ | ||
| function windows_os() |
- 'windos_os()' to 'is_windows()' - 'mkdir_if_not_exist()' to 'create_hooks_dir()' - Replace mkdir hardcode with 'create_hooks_dir()' function
src/Commands/AddCommand.php
Outdated
| if (! is_dir($hookDir)) { | ||
| mkdir($hookDir, 0700, true); | ||
| } | ||
| create_hooks_dir("{$gitDir}/hooks"); |
There was a problem hiding this comment.
last nitpick: I would prefer create_hooks_dir($gitDir); and then the string interpolation will happen in the function. that removes some redundancy. what do you think
There was a problem hiding this comment.
Yes, much better! I did not pay attention to this before 😅, I'll make the change.
- String interpolation within function
|
Done! Much better now! That's why I love the code review 😍... With all these changes, do you consider launching a new release? maybe 2.5. |
|
Yeah I might do a release |
| */ | ||
| function git_version() | ||
| { | ||
| $process = new Process('git describe --tags $(git rev-list --tags --max-count=1)'); |
There was a problem hiding this comment.
This now requires symfony/process as a project dependancy, right?
This PR will: