[5.4] Remove pointless boolean expressions#44885
Conversation
|
I have tested this item ✅ successfully on 196f28d This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
|
I have tested this item ✅ successfully on ec8cc4b This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
|
@janschoenherr Please fix conflicts. Thanks! |
7052276 to
43ac350
Compare
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
HLeithner
left a comment
There was a problem hiding this comment.
For the moment we don't want to merge this change, we will discuss this in the next maintainer meeting.
joomdonation
left a comment
There was a problem hiding this comment.
Thanks for your hard work. I did a code review and see some changes needed. Please see below in each comment (please look at each suggested change carefully and make sure you are agree with the change before committing the suggested change)
administrator/components/com_config/src/Model/ApplicationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_config/src/Model/ApplicationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_content/src/Model/ArticleModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_installer/src/Model/UpdateModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_templates/src/Helper/TemplateHelper.php
Outdated
Show resolved
Hide resolved
|
Thanks @joomdonation for the code review, I agree that's the way to go. |
97e71be to
f26debf
Compare
administrator/components/com_content/src/Model/ArticleModel.php
Outdated
Show resolved
Hide resolved
|
I have tested this item ✅ successfully on 3bf56c2 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
|
@QuyTon Could you please give this another (hopefully final) review? |
|
I have tested this item ✅ successfully on 3bf56c2 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44885. |
|
As agreed today in the maintainers meeting, I've rebased this PR to 5.4-dev. |
|
I really like the improvements work as done in this pr. Unfortunately I fear that it might lead to regressions when that amount of files are touched. If possible it should be split into smaller pieces. When it still is intended to be merged as a whole, then this should be done very early in the 5.4 release cycle to iron out potential regressions. Just my 2cent's. |
Has been discussed and rebased meanwhile.
|
Thanks all |
Remove pointless boolean expressions reported by PhpStorm inspections.
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed