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.

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

ZOOKEEPER-4034: Replace lambda with method reference #1563

Closed
wants to merge 1 commit into from
Closed

ZOOKEEPER-4034: Replace lambda with method reference #1563

wants to merge 1 commit into from

Conversation

lanicc
Copy link
Contributor

@lanicc lanicc commented Dec 23, 2020

No description provided.

Copy link
Member

@maoling maoling left a comment

Choose a reason for hiding this comment

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

+0. It's just another alternative way. It's hard to say it's an optimization?

@lanicc
Copy link
Contributor Author

lanicc commented Dec 28, 2020

+0. It's just another alternative way. It's hard to say it's an optimization?

Yes. It's not really an optimization, just a simplification of the code

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

In my opinion this kind of patches is not worth to be committed.
'Don't fix things that are not broken' is a good motto.

This change does not add much value, but it my affect the execution at runtime.
It may look like nitpicking but it is a broader thought.
We should change code only if there is a good reason do to it.
Even in this case the behaviour may be different for some JVM, so why risk to break things?

@lanicc I encourage you to submit other patches, it is very appreciated here, but it is better to pick up some known issue and try to fix it or if we want to reduce our technical debt probably it is better to pick up a topic (using method references?) and address the problem more globally

@lanicc
Copy link
Contributor Author

lanicc commented Dec 28, 2020

Thank you for your contribution.

In my opinion this kind of patches is not worth to be committed.
'Don't fix things that are not broken' is a good motto.

This change does not add much value, but it my affect the execution at runtime.
It may look like nitpicking but it is a broader thought.
We should change code only if there is a good reason do to it.
Even in this case the behaviour may be different for some JVM, so why risk to break things?

@lanicc I encourage you to submit other patches, it is very appreciated here, but it is better to pick up some known issue and try to fix it or if we want to reduce our technical debt probably it is better to pick up a topic (using method references?) and address the problem more globally

Thank you for your reply, I accept it.

@lanicc lanicc closed this Dec 28, 2020
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.

3 participants