phooq closed pull request #15750: KAFKA-16557: Fix toString of
OffsetFetchRequestState
URL: https://github.com/apache/kafka/pull/15750
--
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 co
kirktrue commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2078159039
I think I would limit it to the request state classes, just to keep the
changes to a minimum
--
This is an automated message from the Apache Git Service.
To respond to the message, ple
phooq commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2078128947
Thanks for the suggestion @kirktrue . I will use `toStringDetails()` then.
Would you suggest I change the methods in the `***Event` classes as well, or
just focus on `RequestState` for this
kirktrue commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077701719
What about `toStringDetails()`?
--
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 specif
kirktrue commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077634416
Thanks for the work on this @phooq!
> I plan to, on top the current changes I have, rename the `toStringBase`
method as `getDetails`
Renaming `toStringBase()` to `getDetails
phooq commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077592945
Thanks so @lianetm ! Hey @kirktrue , I plan to, on top the current changes I
have, rename the `toStringBase` method as `getDetails` for `RequestState`,
does this look okay to you?
T
lianetm commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077166262
Hey @phooq, having a `getDetails` that the inheritors override does achieve
the clarity I was looking for, sounds good.
--
This is an automated message from the Apache Git Service.
To
phooq commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2066909600
Thanks for the feedback @lianetm !
I agree with the point about the back-and-forth jumping between the child
and the parent coming with this implementation, however asking each child
lianetm commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2066727111
Thanks for the info @kirktrue , I see you mentioned it was a "suggested
pattern", and I won't hold a strong position here but still sharing how I see
it (struggling to find a value in it
phooq commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2065444919
Hey @lianetm , @kirktrue Thanks for the feedback and suggestions!
I like @kirktrue 's advice on having a `toString()` override at the base
class while each subclass customized the `to
kirktrue commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2065387420
We've been encouraged to follow the pattern of having the super class
implement `toString()` and add a `toStringBase()`, like this:
```java
public class Parent {
priv
lianetm commented on PR #15750:
URL: https://github.com/apache/kafka/pull/15750#issuecomment-2064223926
Hey @phooq, thanks for taking on this one. High level question about the
motivation. The `toStringBase` defined in the base class `RequestState`
includes vars that all states have, so tha
12 matches
Mail list logo
