vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1639693030
@mimaison can this be merged if all changes look fine?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1632883931
> I took a look at the changes and they look fine. If I understand correctly
we're moving these classes to server-common while we have to keep MirrorMaker.
Then in Kafka 4.0 (and
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1614890801
Thanks @fvaleri . Hmm I see 101 test failures. 92 existing and 9 new.
Atleast the new ones look unrelated..
--
This is an automated message from the Apache Git Service.
To respond
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1614540440
@ruslankrivoshein , I have fixed the checkstyle issues. Also, I believe that
the other comment
[here](https://github.com/apache/kafka/pull/13158#issuecomment-1422555044) has
been
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1596987677
> > I would keep the work that has been done
>
> I agree. This work is already in use in another PR.
I see. In that case, I need to fix the build issue and ask folks for
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1581026296
I see. TBH I haven't followed that migration off-late. I think there are
build failures as well which I haven't addressed yet.
--
This is an automated message from the Apache Git
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1577001257
In that case, would it make sense to close this PR @fvaleri ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1553271619
Sorry I completely missed this. Thanks @fvaleri . @ijuma do you agree with
the reasoning provided by Federico?
--
This is an automated message from the Apache Git Service.
To
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1430997152
> Hi @vamossagar12 this looks good, but I still think we should move
`GetOffsetShellParsingTest` to `TopicPartitionFilterTest` removing any
reference to `GetOffsetShell` (there is
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1422362370
Thanks @fvaleri , I addressed the review comments. Regarding unit tests for
PartitionFilter and TopicPartitionFilter, I checked the tests in
`GetOffsetShellParsingTest` and they are
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-146347
Thanks @fvaleri , I made the changes.
--
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
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1404671786
> TopicPartitionFilter
@fvaleri , sorry about the build failures. I hadn't run the checkstyle for
core on my local before pushing. I will work on the TopicPartitionFilter
vamossagar12 commented on PR #13158:
URL: https://github.com/apache/kafka/pull/13158#issuecomment-1401815286
@fvaleri , plz review this PR.
--
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
13 matches
Mail list logo
