Skip to content

Optimize withAll for primitive bag factories#1376

Merged
donraab merged 1 commit intoeclipse-collections:masterfrom
TheJavaGuy:1372-optimize-withAll-on-bags
Aug 23, 2022
Merged

Optimize withAll for primitive bag factories#1376
donraab merged 1 commit intoeclipse-collections:masterfrom
TheJavaGuy:1372-optimize-withAll-on-bags

Conversation

@TheJavaGuy
Copy link
Contributor

Please review this PR commit by commit starting from the earliest one. I’ve structured commits to be small and self-contained changes.

Every commit has a detailed message that explains why it was done that way. Of course, if you have additional questions I’m more than happy to answer them.

Closes #1372

@Desislav-Petrov
Copy link
Contributor

hi @TheJavaGuy - we usually squash all commits into a single one as per the guide here https://github.com/eclipse/eclipse-collections/blob/master/CONTRIBUTING.md

Copy link
Contributor

@donraab donraab left a comment

Choose a reason for hiding this comment

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

Hi @TheJavaGuy, thank you for the contribution! I have a few changes requested inline. I appreciate all of the detail you have in the commits. It was helpful understanding the issues you encountered using toImmutable as I suggested in the issue. After you make the requested changes, you can squash the commits/rebase and I will review again and approve/merge assuming everything looks good.

@donraab
Copy link
Contributor

donraab commented Aug 22, 2022

Hi @TheJavaGuy, the changes look good. Please update the commit message to added closes with the issue number. Thanks!

Copy link
Contributor

@donraab donraab left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution @TheJavaGuy !

@donraab donraab merged commit 5dd90aa into eclipse-collections:master Aug 23, 2022
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.

Optimize withAll on MutableIntBagFactoryImpl and ImmutableIntBagFactoryImpl

3 participants