? Pending

User tests: Successful: Unsuccessful:

avatar bygiro
bygiro
24 Jun 2019

the modification is self explanatory.

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

avatar bygiro bygiro - open - 24 Jun 2019
avatar bygiro bygiro - change - 24 Jun 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jun 2019
Category Libraries
avatar mbabker
mbabker - comment - 24 Jun 2019

the modification is self explanatory.

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.

avatar brianteeman
brianteeman - comment - 24 Jun 2019

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

avatar alikon
alikon - comment - 24 Jun 2019

please @bygiro add the needed info, it's quite hard to replicate the problem you are solving, without info, even if could be clear what are trying to do from a code review perspective, thanks in advance

avatar Bakual
Bakual - comment - 24 Jun 2019

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?

avatar bygiro
bygiro - comment - 25 Jun 2019

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.

avatar HLeithner
HLeithner - comment - 25 Jun 2019

I'm sorry to say but in php '0' is false

https://3v4l.org/Oi9JH

avatar HLeithner HLeithner - change - 25 Jun 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-06-25 16:24:18
Closed_By HLeithner
Labels Added: ?
avatar HLeithner HLeithner - close - 25 Jun 2019
avatar bygiro
bygiro - comment - 25 Jun 2019

:-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.

avatar Bakual
Bakual - comment - 25 Jun 2019

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.

avatar brianteeman
brianteeman - comment - 25 Jun 2019

@bygiro an apology is due

Add a Comment

Login with GitHub to post a comment