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.

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

Get rid of lambdas in InfinispanClientProducer #10995

Merged
merged 1 commit into from
Jul 28, 2020
Merged

Get rid of lambdas in InfinispanClientProducer #10995

merged 1 commit into from
Jul 28, 2020

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Jul 27, 2020

We avoid lambdas in runtime code because of the implied additional
memory consumption.

@gsmet
Get rid of lambdas in InfinispanClientProducer
6b78bac 
We avoid lambdas in runtime code because of the implied additional
memory consumption.
@gsmet gsmet requested a review from karesti July 27, 2020 14:09
@boring-cyborg boring-cyborg bot added the area/infinispan Infinispan label Jul 27, 2020
@gsmet
Copy link
Member Author

gsmet commented Jul 27, 2020

@geoand I could use another eye on that one too as we can easily introduce bugs in this kind of work.

@gsmet gsmet added this to the 1.7.0 - master milestone Jul 27, 2020
@geoand
Copy link
Contributor

geoand commented Jul 27, 2020

Sure, let me check

geoand
geoand approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Looked at each line and it looks good.

Furthermore my IDE warns when code can be replaced by Optional.ifPresent() and each line did yield a warning, so we have extra confirmation :)

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 27, 2020
@famod
Copy link
Member

famod commented Jul 27, 2020

@gsmet & @geoand Have you ever tried to auto-detect lambdas in runtime code via some tool? This problem seems pop up rather frequently.

@geoand
Copy link
Contributor

geoand commented Jul 27, 2020

We have not, because no one would sit down and fix them all in one shot.

We just tear them out as we find them 😎

@famod
Copy link
Member

famod commented Jul 27, 2020

@geoand

We have not, because no one would sit down and fix them all in one shot.

Yeah, sure. What I had more in mind is to prevent this from even happening.

@geoand
Copy link
Contributor

geoand commented Jul 27, 2020

If we could enforce some kind of rule for runtime code, that would indeed be really useful

@gsmet gsmet merged commit 135cca1 into quarkusio:master Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

@karesti karesti karesti approved these changes

Assignees
No one assigned
Labels
area/infinispan Infinispan triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Milestone
1.7.0.CR1
Development

Successfully merging this pull request may close these issues.
4 participants
@gsmet @geoand @famod @karesti