siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874680923
##########
build.gradle:
##########
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+ include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
Fixed, thanks Chia.
But how should we proceed further?
Like Bytes, Time is exposed and since Time creates/uses Timer both these
should be part of the public APIs (in the next KIP as disussed in KIP-1247),
but we can't make them public without a KIP yet and we can't move them to the
internals package either without avoiding breaking changes since they were
already exposed (leaked) into public APIs.
If we include the entire package in the javadoc (`include
"**/org/apache/kafka/common/utils/*"`), it would make Time and Timer officially
public API without the discussion and design work that was explicitly deferred
during KIP-1247 review.
What's different from #21412 here:
The record package reorganization worked because:
- `TimestampType` was the only exposed class -> stayed where it was and
became public
- All other classes were truly internal -> moved safely
For utils:
- Multiple classes (Bytes, Time, Timer) are exposed -> can't move without
corresponding KIPs
So, considering these constraints, for now till the next KIP is accepted to
make Time and Timer public, it makes sense to include just the `Bytes.java` in
the javadoc:
```gradle
include "**/org/apache/kafka/common/utils/Bytes.java"
```
WDYT? Am I missing a simpler approach here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
