commons-collections icon indicating copy to clipboard operation
commons-collections copied to clipboard

add partitionByChunkSize method in CollectionUtils

Open ramananravi opened this issue 4 years ago • 15 comments

ramananravi avatar Nov 29 '21 15:11 ramananravi

An empty collection shouldn't throw an exception it should just return an empty list...

lespea avatar Nov 29 '21 17:11 lespea

Thanks for the suggestion. Fixed it as suggested now.

ramananravi avatar Nov 29 '21 17:11 ramananravi

There are a lot of shorter ways to do this using Java 8 features: https://stackoverflow.com/questions/30995763/java-8-partition-list

garydgregory avatar Nov 30 '21 19:11 garydgregory

There are a lot of shorter ways to do this using Java 8 features: https://stackoverflow.com/questions/30995763/java-8-partition-list

@garydgregory Thanks a lot for the suggestion. Fixed it now using Java 8 features as suggested. Please let me know in case of any other comments or better suggestions. Thanks again.

ramananravi avatar Dec 01 '21 18:12 ramananravi

@ramananravi Note that Java has a method called subList(), so reusing that name might make sense.

garydgregory avatar Dec 02 '21 17:12 garydgregory

@ramananravi Note that Java has a method called subList(), so reusing that name might make sense.

@garydgregory Are you referring to the method name partitionByChunkSize? The reason for keeping this name is to keep this in sync with ListUtils.partition. Just to add, since partitionByNumOfChunks could be an extended version of this, kept this as partitionByChunkSize instead of just partition. Please let me know if this sounds relevant. Also, wouldn't subList be specific to List (as we are having this util in CollectionUtils)?

ramananravi avatar Dec 02 '21 19:12 ramananravi

@kinow @garydgregory Got some quick questions since this is my first contribution to this repo.

  1. Would this PR be considered for merge as part of upcoming release?
  2. Is there any pending action item from my side on this PR? Thanks in advance.

ramananravi avatar Dec 07 '21 19:12 ramananravi

I'll take a look on Friday.

garydgregory avatar Dec 07 '21 19:12 garydgregory

I'll take a look on Friday.

Any updates on this? Also, in general, what's the PR merge and release cycle? Thanks in advance.

ramananravi avatar Dec 16 '21 13:12 ramananravi

Hi @ramananravi Thanks for the ping. We are all volunteers, the process is slow and from the outside may look random, and I am also busy being on the Log4j team. So patience is key ;-) The releases are fairly infrequent as this is a mature component. We are due for a release indeed but it is not high priority. I'll make it happen, eventually. HTH set expectations.

garydgregory avatar Dec 16 '21 13:12 garydgregory

@garydgregory thanks a lot for the explanation and sorry for the follow-up. Just wanted to know how the merge and release cycle works, just to plan things from my side accordingly. This helps. Thanks again for your time and support on this.

ramananravi avatar Dec 16 '21 16:12 ramananravi

Merging from GitHub is done after a review by an Apache committer like myself based on that person's available time, not much ATM, you might have heard of issues with Log4j.

Releasing is more complex and you can get an idea under the "GENERAL INFORMATION" section in the left bottom menu of https://commons.apache.org/proper/commons-collections/

Gary

On Thu, Dec 16, 2021 at 11:00 AM ramananravi @.***> wrote:

@garydgregory https://github.com/garydgregory thanks a lot for the explanation and sorry for the follow-up. Just wanted to know how the merge and release cycle works, just to plan things from my side accordingly. This helps. Thanks again for your time and support on this.

— Reply to this email directly, view it on GitHub https://github.com/apache/commons-collections/pull/269#issuecomment-995952639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJB6N2HBND3QMSHIEZGLLDURIELHANCNFSM5I7NZUYA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

garydgregory avatar Dec 18 '21 01:12 garydgregory

@garydgregory - extremely sorry for the follow-up. Please let me know if there are any updates on this? Thanks in advance.

ramananravi avatar Jan 19 '22 17:01 ramananravi

@ramananravi If you will fix the conflicting changes, I'll take another look and see if we can get this merged into the main branch.

Claudenw avatar Mar 27 '23 12:03 Claudenw

@ramananravi If you will fix the conflicting changes, I'll take another look and see if we can get this merged into the main branch.

@Claudenw - fixed as suggested. Please help check things and let me know in case of any further questions or comments. Thanks in advance.

ramananravi avatar Apr 05 '23 10:04 ramananravi