Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various changes in libraries/cms #13238

Merged
merged 9 commits into from
Jun 9, 2017
Merged

Various changes in libraries/cms #13238

merged 9 commits into from
Jun 9, 2017

Conversation

frankmayer
Copy link
Contributor

@frankmayer frankmayer commented Dec 15, 2016

Summary of Changes

  • Use modern day type casting
  • Simplify ternary operation
  • Merge unset() calls

This PR is part of a set to try to separate some of the changes done in one of my previous batch PR's for the libraries/cms directory, which is still on hold (#12171).
Once the new set is merged it will hopefully reduce the changes in that PR, so it can be reviewed easier and finally be merged.

The changes in this PR should be fairly easy to review. In hope that this will get merged quickly. ;)

Testing Instructions

None, should not change behavior

Documentation Changes Required

None.

@frankmayer frankmayer mentioned this pull request Jan 5, 2017
6 tasks
# Conflicts:
#	libraries/cms/component/router/rules/standard.php
@frankmayer
Copy link
Contributor Author

Conflicts resolved. Pls check and merge.

@@ -294,7 +294,7 @@ public static function tooltip($selector = '.hasTip', $params = array())
$opt['showDelay'] = (isset($params['showDelay'])) ? (int) $params['showDelay'] : null;
$opt['hideDelay'] = (isset($params['hideDelay'])) ? (int) $params['hideDelay'] : null;
$opt['className'] = (isset($params['className'])) ? $params['className'] : null;
$opt['fixed'] = (isset($params['fixed']) && ($params['fixed'])) ? true : false;
$opt['fixed'] = (isset($params['fixed']) && ($params['fixed']));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove parentheses in ($params['fixed']). Also the outside parentheses can be removed in this line and the other lines too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed. Thanks. Have found one or two other improvements as well. Commit follows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out, some of the other improvements, were already in other PR's which have not been merged yet. It would be good to get the other improvements merged, too.

@Quy
Copy link
Contributor

Quy commented May 29, 2017

I have tested this item ✅ successfully on 372e0d4

Code review.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13238.

@andrepereiradasilva
Copy link
Contributor

I have tested this item ✅ successfully on 372e0d4

on code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13238.

@ghost
Copy link

ghost commented May 31, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 31, 2017
@rdeutz rdeutz added this to the Joomla 3.7.3 milestone Jun 9, 2017
@rdeutz rdeutz merged commit 507febe into joomla:staging Jun 9, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 9, 2017
@frankmayer frankmayer deleted the various-changes-in-libraries-cms branch June 9, 2017 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants