Conversation
25555ae to
869c15e
Compare
869c15e to
974f92a
Compare
DavidGarciaCat
left a comment
There was a problem hiding this comment.
Hey there,
I would like to know if maintainers believe that this update might be useful or not. Most of the methods (or all of them) are self-descriptive so I don't know if devs can get a real benefit of this PR.
Having said that, I believe that there are 2 things that could be reviewed for this PR:
-
I would like to "request" the removal of the
.ideafolder - this is part of your local environment and it doesn't need to be the same than other developers are using. -
Also, I would like to see a bit more of consistency with the annotations.
| @@ -1,2 +1,3 @@ | |||
| /.idea/ | |||
There was a problem hiding this comment.
IMHO the settings folder of your IDE such as .idea for PHPStorm or netbeans for NetBeans should always be included as part of your Global Git Ignore but never in a project's Git Ignore file.
There was a problem hiding this comment.
I agree, although PHPStorm has established itself as a defacto standard.
There was a problem hiding this comment.
I had a few weeks ago, I had a discussion about IDEs with a colleague that basically said he has a love/hate relationship with PHPStorm. Also, until a few months ago, I was working with another developer who moved back to Netbeans due to PHPStorm was ”too slow”.
Although I love PHPStorm, I understand that not everyone wants to use it.
| /** | ||
| * String. | ||
| * | ||
| * @param $value |
There was a problem hiding this comment.
For cases like this, I would suggest one of the following cases:
- Add the annotation as
@param mixed $value - Be consistent with the check that we're expecting to validate - for this case
@param string $value
|
Thank you for your contribution, however, we have decied to merge #118 instead, which provides the annotations that are mentioned here. |
Add annotations to support benefits of modern IDE´s while consuming this package