Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally #7782

izharaazmi · 2015-08-29T15:11:26Z

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally with appropriate deprecation warnings. All functionality of JArrayHelper is already available there, so no point keeping duplicate code.

…lly with appropriate deprecation warnings. All functionality of JArrayHelper is already available there, so no point keeping duplicate code.

roland-d · 2015-09-04T14:46:36Z

@izharaazmi Does this PR need to wait for your fix in joomla-framework/utilities#12 ?

izharaazmi · 2015-09-05T08:04:24Z

@roland-d No. The fix in joomla-framework/utilities#12 is independent of this PR.

Added the missing return statement that caused fromObject method to fail.

izharaazmi · 2015-09-09T12:28:51Z

Kindly let me know If there is something awaited from my part.

roland-d · 2015-09-14T00:08:52Z

@izharaazmi Please provide test instructions that touches this code so users can test that nothing is broken. Code review looks good but that is not enough in this case. Thanks.

izharaazmi · 2015-09-14T08:45:22Z

@roland-d There is no special instruction for this change as I have not made any logical change. No feature added or removed or fixed or improved. The normal tests that applies to JArrayHelper (without this patch) or Joomla\Utilities\ArrayHelper is applicable for this PR as well.
This means the existing unit test cases are well sufficient for this purpose.

However, I might be wrong in saying so, please enlighten me with your perspective.

izharaazmi · 2015-09-29T07:36:44Z

Please advise if I need to make any changes/improvements to this.
Thanks.

roland-d · 2015-10-03T08:21:25Z

@izharaazmi Finally I had some time to test and the proxies are working fine. Only issue I did find is you changed in the logs the names to FUNCTION and this is not working. The value of FUNCTION is always empty, so I would suggest to revert this change. So your point that the existing unit test cases are sufficient is not correct because they do not test the logging.

To test this issue, you will need to enable debugging mode with XDebug for example to see the changes at work. Editing a category will trigger most changed methods except for the pivot and arrayUnique methods, these are only unit tested.

roland-d · 2015-10-03T08:24:43Z

I have tested this item 🔴 unsuccessfully on 7f9ff9d

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

joomla-cms-bot · 2015-10-03T08:56:33Z

This PR has received new commits.

CC: @roland-d

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

izharaazmi · 2015-10-03T09:03:52Z

I just tested the 5 methods in JArrayHelper with object type arguments. I got these lines in my log file. I see the __FUNCTION__ is not empty.

#Fields: datetime   priority clientip   category    message
2015-10-03T08:47:20+00:00   WARNING ::1 deprecated  This method is typehinted to be an array in \Joomla\Utilities\ArrayHelper::toObject.
2015-10-03T08:47:21+00:00   WARNING ::1 deprecated  This method is typehinted to be an array in \Joomla\Utilities\ArrayHelper::toString.
2015-10-03T08:47:21+00:00   WARNING ::1 deprecated  This method is typehinted to be an array in \Joomla\Utilities\ArrayHelper::getColumn.
2015-10-03T08:47:21+00:00   WARNING ::1 deprecated  This method is typehinted to be an array in \Joomla\Utilities\ArrayHelper::pivot.
2015-10-03T08:47:21+00:00   WARNING ::1 deprecated  This method is typehinted to be an array in \Joomla\Utilities\ArrayHelper::sortObjects.

However, I have still changed those 5 lines to fixed string for the method names.
Thanks.

roland-d · 2015-10-03T09:44:06Z

I have tested this item 🔴 unsuccessfully on ad48dac

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

roland-d · 2015-10-03T09:44:42Z

@wilsonge Can you have a look at this too? Looks good to go for me.

izharaazmi · 2015-10-03T09:55:50Z

@roland-d In your last comment you say 'looks good to me', but your recent test status says: tested unsuccessfully on ad48dac. Was there any issue?

roland-d · 2015-10-03T12:40:47Z

@izharaazmi Look at the issue and the test results, I have set the test to successful after your last change.

izharaazmi · 2015-10-03T12:49:56Z

Sorry, but I see the 2nd test too marked as 'tested unsuccessfully' only. May be I'm missing something!

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

roland-d · 2015-10-03T12:57:19Z

I have tested this item ✅ successfully on ad48dac

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

roland-d · 2015-10-03T12:57:33Z

Now it is successful :)

izharaazmi · 2015-10-03T13:01:11Z

Thanks!

anibalsanchez · 2015-11-06T18:31:36Z

I have tested this item ✅ successfully on ad48dac

Applied the patch and worked for a while, no issues so far. Ok

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

zero-24 · 2015-11-06T22:30:01Z

RTC. for 3.5.0 Thanks.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7782.}

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally

…using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.

* re-arrayhelper-min: (2467 commits) 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. remove platform include (joomla#11615) [GitHub Templates] Make headings bigger (joomla#11607) [com_contact] Make ACL core.edit.own work (PR for 11466) (joomla#11503) Small review on docs & code structure in JModelLegacy library classes (joomla#11057) Obviously, this should be an array. (joomla#11610) Don't manually import JPlatform anymore (joomla#10841) Parse preprocess rules from component routers (joomla#8986) Add the correct exception after 11593 merge (was waiting for that merrge) (joomla#11606) Add missing clean line after joomla#9277 (joomla#11605) Deprecate the _PROFILER global var (joomla#10845) Spelling errors (joomla#11604) Moved travis javascript bash file to build/travis like joomla#11600 (joomla#11603) Regression: Fix edit check in backend articles manager, always denying edit after soft deny (joomla#11511) [com_plugins] User not allowed to core.manage? Use 403 php custom exception (instead of a 404 JError) (joomla#11593) [com_newsfeeds] Make ACL core.edit.own work (PR for 11466) (joomla#11502) $result-variable-undefined-given-default-value (joomla#9277) com_banners use exceptions. and not allowed is a 403 (joomla#11418) Frontend & plugins using the autoloader (joomla#10882) New version of PR 6788 (JText::_() Optimizations) (joomla#11235) ...

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

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

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper interna…

22c36b5

…lly with appropriate deprecation warnings. All functionality of JArrayHelper is already available there, so no point keeping duplicate code.

joomla-cms-bot added the PR-staging label Aug 29, 2015

izharaazmi mentioned this pull request Aug 31, 2015

Accept the source data to be array in fromObject #7787

Closed

Missing return statement

7f9ff9d

Added the missing return statement that caused fromObject method to fail.

Change magic constant __FUNCTION__ to fixed method name strings.

ad48dac

zero-24 added this to the Joomla! 3.5.0 milestone Nov 6, 2015

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 6, 2015

rdeutz added a commit that referenced this pull request Nov 16, 2015

Merge pull request #7782 from izharaazmi/arrayhelper-3

62c24a3

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally

rdeutz merged commit 62c24a3 into joomla:staging Nov 16, 2015

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 16, 2015

This was referenced Nov 16, 2015

Revert "Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally" #8455

Merged

removed functions can't included in code coverage #8454

Merged

izharaazmi mentioned this pull request Nov 18, 2015

Minimize JArrayHelper methods toInteger, pivot, arrayUnique #8479

Merged

izharaazmi deleted the arrayhelper-3 branch August 23, 2016 06:17

Uh oh!

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally #7782

Minimizing JArrayHelper by using Joomla\Utilities\ArrayHelper internally #7782

Uh oh!

Conversation

izharaazmi commented Aug 29, 2015

Uh oh!

roland-d commented Sep 4, 2015

Uh oh!

izharaazmi commented Sep 5, 2015

Uh oh!

izharaazmi commented Sep 9, 2015

Uh oh!

roland-d commented Sep 14, 2015

Uh oh!

izharaazmi commented Sep 14, 2015

Uh oh!

izharaazmi commented Sep 29, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

joomla-cms-bot commented Oct 3, 2015

Uh oh!

izharaazmi commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

izharaazmi commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

izharaazmi commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

roland-d commented Oct 3, 2015

Uh oh!

izharaazmi commented Oct 3, 2015

Uh oh!

anibalsanchez commented Nov 6, 2015

Uh oh!

zero-24 commented Nov 6, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants