Skip to content

Minimize JArrayHelper methods toInteger, pivot, arrayUnique #8479

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 22, 2016
Merged

Minimize JArrayHelper methods toInteger, pivot, arrayUnique #8479

merged 3 commits into from
Aug 22, 2016

Conversation

@izharaazmi
Copy link
Contributor

@izharaazmi izharaazmi commented Nov 18, 2015 •
edited by brianteeman
Loading

Minimize JArrayHelper methods toInteger, pivot, arrayUnique by using Joomla\Utilities\ArrayHelper internally.
Leaving (reverting from #7782) other four methods as it was originally for b/c reasons as mentioned in #8455.

Thanks @rdeutz

@izharaazmi
Minimize JArrayHelper methods toInteger, pivot, arrayUnique by …
3a8878e 
…using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.
@izharaazmi
Copy link
Contributor Author

izharaazmi commented Nov 19, 2015

@rdeutz I am not sure if this PR makes any sense after the merge of #8455. Please advise. Do I need to PR afresh for toInteger, pivot and arrayUnique?

@rdeutz
Copy link
Contributor

rdeutz commented Nov 19, 2015

@izharaazmi sure it makes sense #8455 is a revert of #7782, we revert the full PR but there where some good changes in it (#7782) and we should merge them.

@izharaazmi
Copy link
Contributor Author

izharaazmi commented Nov 19, 2015

@rdeutz But In the changes (diff) I see it is not comparing to latest staging branch content, i.e. after the revert I though diff should show up new differences. Am I missing something?

@izharaazmi
Copy link
Contributor Author

izharaazmi commented Nov 19, 2015

Oh I just updated my knowledge and I understand how it works. I was just thinking the wrong way :)
Thanks @rdeutz!

@rdeutz
Copy link
Contributor

rdeutz commented Nov 19, 2015

@izharaazmi you're welcome!

@arbue
Copy link

arbue commented Aug 1, 2016

I have tested this item ✅ successfully on 3a8878e

tested @icampus pizza, bugs & fun.
used the functions toInteger(), pivot() and arrayUnique() from JArrayHelper without patch and with patch and compared the outputs. Outputs were identically.

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

@alimpam
Copy link

alimpam commented Aug 1, 2016

I have tested this item ✅ successfully on 3a8878e

tested @icampus:
besides that JArrayHelper is deprecated, the patch does what it says (Compared class before and after applied patch)

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

@brianteeman
Copy link
Contributor

brianteeman commented Aug 1, 2016

RTC

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 1, 2016
@roland-d roland-d modified the milestones: Joomla 3.6.2, Joomla 3.7.0 Aug 2, 2016
@wilsonge
Copy link
Contributor

wilsonge commented Aug 13, 2016

@izharaazmi Can you rebase this please?

@izharaazmi
Minimize JArrayHelper methods toInteger, pivot, arrayUnique by …
baaae99 
…using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.
@izharaazmi izharaazmi closed this Aug 15, 2016
@izharaazmi izharaazmi deleted the re-arrayhelper-min branch August 15, 2016 17:01
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 15, 2016
@brianteeman brianteeman removed this from the Joomla 3.7.0 milestone Aug 15, 2016
@izharaazmi izharaazmi reopened this Aug 15, 2016
@izharaazmi
Copy link
Contributor Author

izharaazmi commented Aug 15, 2016

@wilsonge Done. Please verify.

zero-24
zero-24 reviewed Aug 15, 2016
View reviewed changes
libraries/joomla/utilities/arrayhelper.php
* @return array
*
* @see https://secure.php.net/manual/en/function.array-unique.php
* @see http://php.net/manual/en/function.array-unique.php
Copy link
Contributor

@zero-24 zero-24 Aug 15, 2016

Choose a reason for hiding this comment

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

This should stay to be https or do we have any reason to set it to http?

Copy link
Contributor Author

@izharaazmi izharaazmi Aug 15, 2016 via email

Choose a reason for hiding this comment

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

@brianteeman
Copy link
Contributor

brianteeman commented Aug 22, 2016

Setting back to RTC as it was just a rebase @wilsonge

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 22, 2016
@rdeutz rdeutz added this to the Joomla 3.6.3 milestone Aug 22, 2016
@rdeutz rdeutz merged commit 4363a8d into joomla:staging Aug 22, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 22, 2016
zero-24 added a commit to zero-24/joomla-cms that referenced this pull request Aug 22, 2016
@zero-24
Back to https after joomla#8479
e7fd0e5
@zero-24 zero-24 mentioned this pull request Aug 22, 2016
Merged
jeckodevelopment added a commit that referenced this pull request Aug 22, 2016
@jeckodevelopment
Merge pull request #11745 from zero-24/patch-16
714191f 
Back to https after #8479
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
@izharaazmi @roland-d
Minimize JArrayHelper methods toInteger, pivot, arrayUnique (jo…
a5a8267 
…omla#8479)

* Minimize JArrayHelper methods `toInteger`, `pivot`, `arrayUnique` by using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.

* Minimize JArrayHelper methods `toInteger`, `pivot`, `arrayUnique` by using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
@zero-24 @roland-d
Back to https after joomla#8479
a55bd01
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
@izharaazmi @roland-d
Minimize JArrayHelper methods toInteger, pivot, arrayUnique (jo…
5beeec6 
…omla#8479)

* Minimize JArrayHelper methods `toInteger`, `pivot`, `arrayUnique` by using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.

* Minimize JArrayHelper methods `toInteger`, `pivot`, `arrayUnique` by using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
@zero-24 @roland-d
Back to https after joomla#8479
dd274da
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 3.6.3

Development

Successfully merging this pull request may close these issues.

9 participants

@izharaazmi @rdeutz @arbue @alimpam @brianteeman @wilsonge @zero-24 @roland-d @joomla-cms-bot