[#12228] - Performance 4 (plugins)
- Fixed in Code Base
- 8 Jun 2017
- Medium
- Build: staging
- # 12228
- Diff
- frankmayer:Performance_4
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success JTracker/HumanTestResults Human Test Results: 3 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
[UPDATED (1.6.2017) - see third paragraph]
Performance (and a bit of cleanup) Batch #4
The changes in this batch are all in files under plugins.
This PR modifies code to be a bit more performant and also does some cleanup.
I have mostly done work on low hanging fruit. There are still other ways of improving, but whose involve more deep research in the code and probably more drastic changes in order to be implemented.
[Update 6.1.2017]
In order to lighten this PR up, I have introduced 5 sub-PRs, which are easier to digest and mostly with very specific changes. In order to continue with this one, those ones should be tested/reviewed/merged first. After that, I can resolve the conflicts and there should finally be only a few changes left.
- 1) Replaced unnecessary double quotes in plugins #13211
- 2) Removed unnecessary parentheses in plugins #13212
- 3) Various changes in plugins #13213
- 4) Type safe string comparison of strings in plugins #13271
- 5) Type safe comparison in plugins - second iteration #13272
[Update 1.6.2017]
All Sub-PRs of this PR have now been merged. That means a lot less changes to be reviewed
Please review.
Thank you!
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Category | ⇒ | Front End Plugins |
I have tested this item
Code review.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12228.
All the sub PR's are now merged. Do you want to resolve conflicts here for the last few things?
Conflicts resolved...
Hmm, this has still lots of different changes. Will do one or more sub PR's to lighten this up a little more.
Edited initial post to show which sub-PRs still have to go through, in order to make this one easier to digest.
It would be great if the initial big batch PRs of mine could make it into alpha2. Thanks!
| Category | Front End Plugins | ⇒ | Administration com_admin com_associations com_banners com_categories com_config com_contact com_content com_contenthistory com_fields com_finder |
Pls don't review this yet. There was a git problem. Need to resolve first.
| Category | Administration com_admin com_associations com_banners com_categories com_config com_contact com_content com_contenthistory com_fields com_finder | ⇒ | Front End Plugins |
All Sub-PRs of this PR have now been merged. That means a lot less changes to be reviewed
Please review.
Thank you!
next in my list of tests - tomorrow probably
@Quy would you be so kind and check the changes of your reviews for this one, too? So maybe we can get this RTC as soon as @andrepereiradasilva also has time to check it?
Thanks guys!!!
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12228.
| Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
@andrepereiradasilva set Status back on Pending?
@franz-wohlkoenig Let me just fly over those comments and I'll come back to you.
| Labels |
Added:
?
|
||
@franz-wohlkoenig @andrepereiradasilva updated according to reviewer's comments.
Please check and RTC.
Thanks!!
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12228.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-08 06:38:58 |
| Closed_By | ⇒ | rdeutz |
I have tested this item✅ successfully on 42a5a54
code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12228.