User tests: Successful: Unsuccessful:
the modification is self explanatory.
Pull Request for Issue # .
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
just saying "read the code and figure it out" is going to result in most people just ignoring the pull request.
which is exactly what I did
I think while the code itself could be considered cleaner, there is no real bug to solve here and the statement in the title ("allowAdd never trigged") is wrong. In PHP '0'
evaluates to boolean false
exactly the same as 0
and thus it will trigger allowAdd
just fine.
The behaviour before and after this PR will be the same.
So it would be indeed interesting why the OP wanted this PR. Was there an actual Bug to solve or did he just stumble over this particular line?
come on guys, seriously, we are on github because we are supposed to be programmers, I assume to deal with programmers, those few lines are really self explanatory.
the line:
$this->allowAdd($data);
will never be executed, because a string '0' is never considered FALSE.
To be honest I found and fixed so many bugs on joomla, and to avoid the pull requests I write my own class and override the joomla code, this time I wanted to give another try, but I see nothing changes in this repository, and that's why joomla development is so slow! and sometimes obsolete.
it's simply annoying the approach some of you have here.
Btw, That explains a lot of things.
I'm sorry to say but in php '0' is false
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-06-25 16:24:18 |
Closed_By | ⇒ | HLeithner | |
Labels |
Added:
?
|
:-D sorry guys, as expected the murphy's law is always there!
thanks @HLeithner I really didn't think about that, yes, you are right! it's false.
come on guys, seriously, we are on github because we are supposed to be programmers, I assume to deal with programmers, those few lines are really self explanatory.
That's a false assumption. There are lots of people here testing PRs which are not programmers. That's why we expect an explanation and testing instructions. Also it's a sign of respect to the ones reading your PR when you explain it, otherwise all others will have to figure out what you did - and that is wasting everyones time.
While it may be self explanatory for you, there is a reason there is a pull request template. It is expected you explain why the change is necessary so that others can understand the problem it is solving, just saying "read the code and figure it out" is going to result in most people just ignoring the pull request.