MINOR: Clean up core metrics, migration, network, raft, security, serializer, tools, utils, and zookeeper modules#15279
Conversation
| import org.apache.kafka.common.utils.Utils | ||
| import org.apache.kafka.server.util.{CommandDefaultOptions, CommandLineUtils} | ||
|
|
||
| import java.util |
There was a problem hiding this comment.
I've got a PR to move this to the tools module: #15274 so I'm not sure it's worth updating it.
There was a problem hiding this comment.
I can review your PR first, and then rebase this one, so this class is gone :)
There was a problem hiding this comment.
Rebased against trunk. Feel free to review whenever you have time @mimaison. Thanks!
| @volatile var callbackRequestCompleteTimeNanos: Option[Long] = None | ||
|
|
||
| val session = Session(context.principal, context.clientAddress) | ||
| val session: Session = Session(context.principal, context.clientAddress) |
There was a problem hiding this comment.
I see sometimes we add the type for val and var definition and sometimes we don't am just curious what is the pattern we are opting in for here?
There was a problem hiding this comment.
I might have missed some spots where this should be applied but the Scala idiomatic approach to this is:
- public API should be explicitly typed: this applies to
vals,vars anddefs for example. Like the case you comment on. - private definitions or variables don't need to be typed. Thanks to Scala type inference many
defs,vars andvals can omit their explicit type making it easy to refactor things. Please note the "don't need to", you can still explicitly type those.
Let me know if this answers your question.
There was a problem hiding this comment.
The changes made in this PR reflect the current (as per 2.12 and 2.13) idiomatic way to write Scala. For example:
Boolean parameters should be named, idempotent methods that are parameterless can be omit the parenthesis, side-effecting parameterless methods should include the parenthesis, and so on
This PR cleans up: metrics, migration, network, raft, security, serializer, tools, utils, and zookeeper package classes Mark methods and fields private where possible Annotate public methods and fields Remove unused classes and methods Make sure Arrays are not printed with .toString Optimize minor warnings
jlprat
force-pushed
the
MINOR-cleanup-core-part3
branch
from
e89a791 to
4c489eb
Compare
|
Checked that all tests pass locally and are flaky tests. |
This PR cleans up: metrics, migration, network, raft, security, serializer, tools, utils, and zookeeper package classes Mark methods and fields private where possible Annotate public methods and fields Remove unused classes and methods Make sure Arrays are not printed with .toString Optimize minor warnings Reviewers: Mickael Maison <mickael.maison@gmail.com>
This PR cleans up: metrics, migration, network, raft, security, serializer, tools, utils, and zookeeper package classes Mark methods and fields private where possible Annotate public methods and fields Remove unused classes and methods Make sure Arrays are not printed with .toString Optimize minor warnings Reviewers: Mickael Maison <mickael.maison@gmail.com>
3 participants
Second follow up from #15252
This PR cleans up: metrics, migration, network, raft, security, serializer, tools, utils, and zookeeper package classes
Committer Checklist (excluded from commit message)