- Notifications
You must be signed in to change notification settings - Fork3.4k
Cleanup around ServerInfo#27664
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Cleanup around ServerInfo#27664
Conversation
We do not depend on equality being done only on nodeVersion andenvironment anywhere in the codebase.
client/trino-client/src/main/java/io/trino/client/ServerInfo.java OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
f9d7cf5 to987cd13Compare987cd13 to868cfbdCompare| if (this ==o) { | ||
| returntrue; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why? It's a good short-circuit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it's interesting that IntelliJ-generated equals doesn't include it (it looks exactly as the code in this PR).
I typically would use IntelliJ-generated unless there is a reason not to.
Wonder why they don't include this short-circuit check. Is it because it adds one more branch to the code?
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: